[PATCH] D47607: [libcxx] Almost fix some UB in and
EricWF added a comment. I should have asked, have we actually seen a midcompile caused by this? Is there a reproducer? Or is this purerly speculative? Repository: rCXX libc++ https://reviews.llvm.org/D47607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > leonardchan wrote: > > leonardchan wrote: > > > ebevhan wrote: > > > > leonardchan wrote: > > > > > rsmith wrote: > > > > > > jfb wrote: > > > > > > > This seems weird because Targets which don't have these values > > > > > > > for the non-Accum versions will have .e.g. `sizeof(short) != > > > > > > > sizeof(short _Accum)`. Is there a point in ever having `_Accum` > > > > > > > differ in size, width, and alignment from the underlying type? If > > > > > > > not, can you set these values after the sub-target has specified > > > > > > > its preferences? > > > > > > I'm uncomfortable about opting all targets into this behavior with > > > > > > these default values; this will result in an ABI break if later a > > > > > > target updates these to the correct values. A per-target > > > > > > `AccumSupported` flag would help substantially. But this is OK for > > > > > > the short term while you're still working on the feature. > > > > > > > > > > > > We'll also need the target to inform us of the number of integer > > > > > > and fractional bits for each such type. > > > > > The integral and fractional bits will be set in the target and is > > > > > available in a later patch. > > > > > We'll also need the target to inform us of the number of integer and > > > > > fractional bits for each such type. > > > > > > > > I believe the only one that is needed is for the number of fractional > > > > bits; the number of integer bits is implied by the difference between > > > > the type width and fractional bits. I think I mention this in one of > > > > the other patches. > > > > > > > > > > > You're right. I was stuck in the mindset that we would be providing an > > > integral and fractional value. > > > The integral and fractional bits will be set in the target and is > > > available in a later patch. > > > > I mean just the fractional bits since the integral will just be the bit > > width minus fractional. > You're assuming that all bits will be either integral or fractional bits (and > in particular that there are no padding bits). I don't know if that's > actually going to be true for all target ABIs, but I suppose it's OK to make > this assumption until it's proven wrong by some actual target. It is actually the case for us (downstream) that the unsigned types should have one bit of padding, however, this is a special case. The spec says "Each unsigned fract type has either the same number of fractional bits as, or one more fractional bit than, its corresponding signed fract type." and also under 'recommended practice', "Each signed accum type has the same number of fractional bits as either its corresponding signed fract type or its corresponding unsigned fract type." So I think the approach would be that the integral bits are implied from the fractional bits and the width, except in the unsigned case where the MSB is a padding bit. If there is a target that really does want the unsigned types to have an extra bit of precision it could be added as a flag, but I don't really see the benefit of it. The consistency benefit of having the unsigned types have the same scaling factor as the signed ones outweighs the downsides. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents
ebevhan added inline comments.
Comment at: include/clang/AST/ASTContext.h:2882
+
+ QualType getCorrespondingSaturatedType(const QualType &Ty) const;
};
This probably belongs up near the other predicates.
Also I think it's more common to simply pass `QualType` instead of a `const
QualType&`.
Comment at: lib/Sema/DeclSpec.cpp:1123
+if (!(TypeSpecType == TST_accum || TypeSpecType == TST_fract)) {
+ S.Diag(TSSatLoc, diag::err_invalid_saturation_spec)
+ << getSpecifierName((TST)TypeSpecType, Policy);
leonardchan wrote:
> ebevhan wrote:
> > Handling this case here means that placing _Sat on something other than
> > exactly a fixed-point type is a parsing error rather than a semantic error.
> > How does this handle _Sat on sugared types? Should _Sat on things like
> > typedefs work?
> >
> > typedef _Fract myfract;
> > _Sat myfract F;
> >
> > The primary issue (and this is one that we have encountered as well) is
> > that you cannot have a true _Sat typedef since _Sat only exists as part of
> > builtin types. You need to desugar/canonicalize the type and then do
> > getCorrespondingSaturatingType (or have getCorrespondingSaturatingType look
> > at the canonical type internally).
> I think _Sat is analogous to _Complex where it only works with specific
> builtin types, albeit the only builtin type _Complex doesn't work with is
> _Bool.
>
> Currently this example would throw the error `'_Sat' specifier is only valid
> on '_Fract' or '_Accum', not 'type-name'` which is similar to what _Complex
> does when paired with a typedef:
>
> ```
> typedef double mydouble;
> mydouble _Complex D; // _Complex type-name' is invalid
> ```
>
> I don't see this as a big problem right now, but am willing to come back to
> this in the future if it becomes more urgent. For now, I added a test that
> asserts this error is thrown.
That sounds reasonable. I have no strong opinions on it and I don't think we
use the functionality anyway.
Comment at: lib/Sema/SemaType.cpp:1612
+ // Only fixed point types can be saturated
+ if (DS.isTypeSpecSat()) {
+if (!IsFixedPointType) {
The braces aren't needed.
Repository:
rC Clang
https://reviews.llvm.org/D46911
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47620: Remove llvm::Triple argument from get***Personality() functions
aheejin created this revision.
aheejin added a reviewer: sbc100.
Herald added a subscriber: cfe-commits.
Because `llvm::Triple` can be derived from `TargetInfo`, it is simpler
to take only `TargetInfo` argument.
Repository:
rC Clang
https://reviews.llvm.org/D47620
Files:
lib/CodeGen/CGException.cpp
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -114,8 +114,9 @@
const EHPersonality
EHPersonality::GNU_Wasm_CPlusPlus = { "__gxx_wasm_personality_v0", nullptr };
-static const EHPersonality &getCPersonality(const llvm::Triple &T,
+static const EHPersonality &getCPersonality(const TargetInfo &Target,
const LangOptions &L) {
+ const llvm::Triple &T = Target.getTriple();
if (L.SjLjExceptions)
return EHPersonality::GNU_C_SJLJ;
if (L.DWARFExceptions)
@@ -127,11 +128,12 @@
return EHPersonality::GNU_C;
}
-static const EHPersonality &getObjCPersonality(const llvm::Triple &T,
+static const EHPersonality &getObjCPersonality(const TargetInfo &Target,
const LangOptions &L) {
+ const llvm::Triple &T = Target.getTriple();
switch (L.ObjCRuntime.getKind()) {
case ObjCRuntime::FragileMacOSX:
-return getCPersonality(T, L);
+return getCPersonality(Target, L);
case ObjCRuntime::MacOSX:
case ObjCRuntime::iOS:
case ObjCRuntime::WatchOS:
@@ -153,9 +155,9 @@
llvm_unreachable("bad runtime kind");
}
-static const EHPersonality &getCXXPersonality(const llvm::Triple &T,
- const LangOptions &L,
- const TargetInfo &Target) {
+static const EHPersonality &getCXXPersonality(const TargetInfo &Target,
+ const LangOptions &L) {
+ const llvm::Triple &T = Target.getTriple();
if (L.SjLjExceptions)
return EHPersonality::GNU_CPlusPlus_SJLJ;
if (L.DWARFExceptions)
@@ -174,31 +176,30 @@
/// Determines the personality function to use when both C++
/// and Objective-C exceptions are being caught.
-static const EHPersonality &getObjCXXPersonality(const llvm::Triple &T,
- const LangOptions &L,
- const TargetInfo &Target) {
+static const EHPersonality &getObjCXXPersonality(const TargetInfo &Target,
+ const LangOptions &L) {
switch (L.ObjCRuntime.getKind()) {
// In the fragile ABI, just use C++ exception handling and hope
// they're not doing crazy exception mixing.
case ObjCRuntime::FragileMacOSX:
-return getCXXPersonality(T, L, Target);
+return getCXXPersonality(Target, L);
// The ObjC personality defers to the C++ personality for non-ObjC
// handlers. Unlike the C++ case, we use the same personality
// function on targets using (backend-driven) SJLJ EH.
case ObjCRuntime::MacOSX:
case ObjCRuntime::iOS:
case ObjCRuntime::WatchOS:
-return getObjCPersonality(T, L);
+return getObjCPersonality(Target, L);
case ObjCRuntime::GNUstep:
return EHPersonality::GNU_ObjCXX;
// The GCC runtime's personality function inherently doesn't support
// mixed EH. Use the ObjC personality just to avoid returning null.
case ObjCRuntime::GCC:
case ObjCRuntime::ObjFW:
-return getObjCPersonality(T, L);
+return getObjCPersonality(Target, L);
}
llvm_unreachable("bad runtime kind");
}
@@ -220,9 +221,10 @@
return getSEHPersonalityMSVC(T);
if (L.ObjC1)
-return L.CPlusPlus ? getObjCXXPersonality(T, L, Target)
- : getObjCPersonality(T, L);
- return L.CPlusPlus ? getCXXPersonality(T, L, Target) : getCPersonality(T, L);
+return L.CPlusPlus ? getObjCXXPersonality(Target, L)
+ : getObjCPersonality(Target, L);
+ return L.CPlusPlus ? getCXXPersonality(Target, L)
+ : getCPersonality(Target, L);
}
const EHPersonality &EHPersonality::get(CodeGenFunction &CGF) {
@@ -318,8 +320,7 @@
return;
const EHPersonality &ObjCXX = EHPersonality::get(*this, /*FD=*/nullptr);
- const EHPersonality &CXX =
- getCXXPersonality(getTarget().getTriple(), LangOpts, getTarget());
+ const EHPersonality &CXX = getCXXPersonality(getTarget(), LangOpts);
if (&ObjCXX == &CXX)
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47614: [WebAssembly] Hide new Wasm EH behind its feature flag
aheejin marked an inline comment as done. aheejin added inline comments. Comment at: lib/CodeGen/CGException.cpp:322 const EHPersonality &CXX = - getCXXPersonality(getTarget().getTriple(), LangOpts); + getCXXPersonality(getTarget().getTriple(), LangOpts, getTarget()); if (&ObjCXX == &CXX) sbc100 wrote: > If the triple can be derived from target why not just pass that target? Done in D47620. Thanks! Repository: rC Clang https://reviews.llvm.org/D47614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm with just a few more nits.
Comment at: clang-doc/BitcodeWriter.cpp:484
#undef EMITINFO
Nit: `EMITINFO` is a bit confusing with `writeInfo`. Are they doing the same
thing (`emit` sounds like `write`)? You might want to rename either of them.
While you are here, it might also make sense to clear up the MACRO :)
Comment at: clang-doc/Representation.cpp:57
+return llvm::make_error("Unexpected info type.\n",
+ std::error_code());
+ }
nit: `std::error_code()` is usually used to indicate no error. You could use
`llvm::inconvertibleErrorCode()`.
Comment at: clang-doc/tool/ClangDocMain.cpp:181
+doc::writeInfo(I.get(), Buffer);
+ if (DumpResultToFile("bc", Group.getKey() + ".bc", Buffer))
+return 1;
juliehockett wrote:
> ioeric wrote:
> > juliehockett wrote:
> > > ioeric wrote:
> > > > (Sorry that I might be missing context here.)
> > > >
> > > > Could you please explain the incentive for dumping each info group to
> > > > one bc file? If the key identifies a symbol (e.g. USR), wouldn't this
> > > > result in a bitcode file being created for each symbol? This doesn't
> > > > seem very scalable.
> > > Yes, it would. This is mostly for debugging, since there's not really any
> > > tools outside the clang system that would actually want/be able to use
> > > this information.
> > Ok, is there any plan to convert intermediate result to final result and
> > output in a more accessible format? If so, please add a `FIXME` somewhere
> > just to be clearer.
> That's what the next patch set is (to generate YAML).
Sure. I think a `FIXME` should be in place if the feature is not already there.
Comment at: clang-doc/tool/ClangDocMain.cpp:68
+"format",
+llvm::cl::desc("Format for outputted docs (Current options are yaml)."),
+llvm::cl::init("yaml"), llvm::cl::cat(ClangDocCategory));
nit: s/current option/default option/
Comment at: clang-doc/tool/ClangDocMain.cpp:68
+"format",
+llvm::cl::desc("Format for outputted docs (Current options are yaml)."),
+llvm::cl::init("yaml"), llvm::cl::cat(ClangDocCategory));
ioeric wrote:
> nit: s/current option/default option/
consider using enum type options. Example:
https://github.com/llvm-mirror/clang-tools-extra/blob/master/include-fixer/tool/ClangIncludeFixer.cpp#L85
Comment at: clang-doc/tool/ClangDocMain.cpp:178
+if (!Reduced)
+ llvm::errs() << llvm::toString(Reduced.takeError());
+
Shouldn't we `continue` to the next group if reduction goes wrong for the
current group?
https://reviews.llvm.org/D43341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR
tkrupa added a comment. Mask scalar case is closed and doesn't have any effects on this revision. Besides, I resolved issues connected to lowering scalar sqrt intrinsics without rounding (that is, if https://reviews.llvm.org/D47621 is accepted). Should I add them here to have everything sqrt in one place or upstream this and add them to a new revision? Repository: rC Clang https://reviews.llvm.org/D41168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker
baloghadamsoftware added a comment. Did the tests execute? I am not sure. First problem is the the container may become dead before the iterator, so its `Begin` and `End` symbols may be inaccessible. This is easy to solve by marking the container of the iterator as live. However, there is a second problem that disables correct tracking of iterators: memory regions are marked as dead, however there are `LazyCompoundVal`s referring to them. Is this maybe a bug in `SymbolReaper`? Repository: rC Clang https://reviews.llvm.org/D47417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
balazske updated this revision to Diff 149412.
balazske added a comment.
- Added comment, renamed beginSourceFile, removed check for PP.
Check for PP is removed because it is allowed to be nullptr.
Repository:
rC Clang
https://reviews.llvm.org/D47445
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
ASTContext &FromCtx = FromAST->getASTContext(),
&ToCtx = ToAST->getASTContext();
+ FromAST->enableSourceFileDiagnostics();
+ ToAST->enableSourceFileDiagnostics();
+
ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -172,7 +175,9 @@
: Code(Code), FileName(FileName),
Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
this->FileName)),
- TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+ TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+ Unit->enableSourceFileDiagnostics();
+}
};
// We may have several From contexts and related translation units. In each
@@ -214,6 +219,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->enableSourceFileDiagnostics();
ASTContext &FromCtx = FromTU.Unit->getASTContext(),
&ToCtx = ToAST->getASTContext();
@@ -261,6 +267,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->enableSourceFileDiagnostics();
return ToAST->getASTContext().getTranslationUnitDecl();
}
@@ -274,6 +281,7 @@
// Build the AST from an empty file.
ToAST =
tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+ ToAST->enableSourceFileDiagnostics();
}
// Create a virtual file in the To Ctx which corresponds to the file from
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -274,6 +274,12 @@
this->PP = std::move(PP);
}
+void ASTUnit::enableSourceFileDiagnostics() {
+ assert(getDiagnostics().getClient() && Ctx &&
+ "Bad context for source file");
+ getDiagnostics().getClient()->BeginSourceFile(Ctx->getLangOpts(), PP.get());
+}
+
/// Determine the set of code-completion contexts in which this
/// declaration should be shown.
static unsigned getDeclShowContexts(const NamedDecl *ND,
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -438,6 +438,15 @@
void setASTContext(ASTContext *ctx) { Ctx = ctx; }
void setPreprocessor(std::shared_ptr pp);
+ /// Enable source-range based diagnostic messages.
+ ///
+ /// If diagnostic messages with source-range information are to be expected
+ /// and AST comes not from file (e.g. after LoadFromCompilerInvocation) this
+ /// function has to be called.
+ /// The function is to be called only once and the AST should be associated
+ /// with the same source file afterwards.
+ void enableSourceFileDiagnostics();
+
bool hasSema() const { return (bool)TheSema; }
Sema &getSema() const {
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
ASTContext &FromCtx = FromAST->getASTContext(),
&ToCtx = ToAST->getASTContext();
+ FromAST->enableSourceFileDiagnostics();
+ ToAST->enableSourceFileDiagnostics();
+
ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -172,7 +175,9 @@
: Code(Code), FileName(FileName),
Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
this->FileName)),
- TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+ TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+ Unit->enableSourceFileDiagnostics();
+}
};
// We may have several From contexts and related translation units. In each
@@ -214,6 +219,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->enableSourceFileDiagnostics();
ASTContext &FromCtx = FromTU.Unit->getASTContext(),
&ToCtx = ToAST->getASTContext();
@@ -261,6 +267,7 @@
ToCode = ToSrcCode;
assert(!ToAST);
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, Out
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
balazske added a comment. From API point of view if there is a `enableSourceFileDiagnostics` there should be a `disableSourceFileDiagnostics` too (that calls the `EndSourceFile`). But I am not sure how and if to use it at all. In the unit tests it is not needed, the ASTUnit contains a single entity for the "To" context. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47537: [clang-tools-extra] Cleanup documentation routine
ioeric added inline comments. Comment at: clang-tools-extra/docs/index.rst:27 pp-trace - clang-rename clangd It seems that the clang-rename tool is still in the extra repository. I think we should probably "advertise" `clang-rename` as part of clang-refactor in the future. That said, we might want to wait until we have proper documentation for clang-refactor in clang. WDYT? https://reviews.llvm.org/D47537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests
SjoerdMeijer added inline comments.
Comment at: test/Sema/aarch64-neon-fp16-ranges.c:1
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon
-fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding
-fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon
-fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding
-fsyntax-only -verify %s
Nit: target feature fullfp16 implies ARMv8 FP, so I think you can remove +neon;
just a tiny optimisation to make the command line shorter (same below).
Comment at: test/Sema/aarch64-neon-fp16-ranges.c:39
+
+void test_vcvt_su_f(int64_t a){
+ vcvth_n_s16_f16(a, 1);
why is this is 'a' an int64_t? Should this not be float16_t?
https://reviews.llvm.org/D47592
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47592: [AArch64] Corrected FP16 Intrinsic range checks in Clang + added Sema tests
SjoerdMeijer added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:1409
- switch (BuiltinID) {
-#define GET_NEON_OVERLOAD_CHECK
-#include "clang/Basic/arm_neon.inc"
Why do we need to remove this?
Comment at: lib/Sema/SemaChecking.cpp:1462
-#define GET_NEON_IMMEDIATE_CHECK
-#include "clang/Basic/arm_neon.inc"
-#include "clang/Basic/arm_fp16.inc"
And also this one?
https://reviews.llvm.org/D47592
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44954: [clangd] Add "member" symbols to the index
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
malaperle wrote:
> sammccall wrote:
> > ioeric wrote:
> > > s/this is symbol/this symbol/?
> > Sorry, I hadn't seen this patch until now.
> > When it was part of the `workspace/symbol` patch, I was the other person
> > concerned this was too coupled to existing use cases and preferred
> > something more descriptive.
> >
> > I dug up an analysis @hokein and I worked on. One concluseion we came to
> > was that we thought results needed by `completion` were a subset of what
> > `workspace/symbol` needs, so a boolean would work. It seemed a bit ad-hoc
> > and fragile though.
> >
> > The cases we looked at were:
> >
> > ||private|member|local|primary template|template specialization|nested in
> > template|
> > | code complete |N|N|N|Y|N|N|
> > | workspace/symbol |Y|Y|N|Y|Y|Y|
> > | go to defn |Y|Y|?|?|?|?|
> > (Y = index should return it, N = index should not return it, ? = don't care)
> >
> > So the most relevant information seems to be:
> > - is this private (private member, internal linkage, no decl outside cpp
> > file, etc)
> > - is this nested in a class type (or template)
> > - is this a template specialization
> >
> > I could live with bundling these into a single property (though they seem
> > like good ranking signals, and we'd lose them for that purpose), it will
> > certainly make a posting-list based index more efficient.
> >
> > In that case I think we should have canonical documentation *somewhere*
> > about exactly what this subset is, and this field should reference that.
> > e.g. `isIndexedForCodeCompletion()` in `CodeComplete.h` with docs and
> > `IsIndexedForCodeCompletion` here. (Please avoid "global" as it has too
> > many different meanings - here we mean "index-based").
> OK, I added documentation on the SymbolCollector (which was outdated) about
> what kinds of symbols are collected, with a reference to shouldFilterDecl.
> The subset is documented on isIndexedForCodeCompletion(), also referenced
> from the Symbol field. Was that more or less what you meant?
> I could live with bundling these into a single property (though they seem
> like good ranking signals, and we'd lose them for that purpose), it will
> certainly make a posting-list based index more efficient.
FWIW, I think we could still add those signals when we need them in the future.
It seems reasonable to me for the code completion flag to co-exist with ranking
signals that could potentially overlap.
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
+TEST(CompletionTest, Enums) {
+ EXPECT_THAT(completions(R"cpp(
malaperle wrote:
> ioeric wrote:
> > It's not clear to me what the following tests (`Enums`,
> > `AnonymousNamespace`, `InMainFile`) are testing. Do they test code
> > completion or symbol collector? If these test symbol collection, they
> > should be moved int SymbolCollectorTest.cpp
> They are testing that code completion works as intended regardless of how
> symbol collector is implemented. It's similar to our previous discussion in
> D44882 about "black box testing". I can remove them but it seems odd to me to
> not have code completion level tests for all cases because we assume that it
> will behave a certain way at the symbol collection and querying levels.
FWIW, I am not against those tests at all; increasing coverage is always a nice
thing to do IMO. I just thought it would make more sense to add them in a
separate patch if they are not related to changes in this patch; I found
unrelated tests a bit confusing otherwise.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:141
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+Args.push_back(TestFileName);
+
malaperle wrote:
> This allows to override the "-xc++" with something else, i.e. -xobjective-c++
I think this could also be a comment in the code :)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.
ebevhan updated this revision to Diff 149415.
ebevhan edited the summary of this revision.
ebevhan added a comment.
Changed ArrayIndexTy back to LongLongTy and reverted the test change.
https://reviews.llvm.org/D46944
Files:
lib/StaticAnalyzer/Core/ProgramState.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/index-type.c
Index: test/Analysis/index-type.c
===
--- test/Analysis/index-type.c
+++ test/Analysis/index-type.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10
-analyzer-checker=core,alpha.security.ArrayBoundV2 -DM32 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core,alpha.security.ArrayBoundV2
-Wno-implicit-function-declaration -verify %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10
-analyzer-checker=core,alpha.security.ArrayBoundV2
-Wno-implicit-function-declaration -DM32 -verify %s
// expected-no-diagnostics
#define UINT_MAX (~0u)
@@ -36,4 +36,23 @@
*ptr = 42; // no-warning
}
+#define SIZE 4294967296
+
+static unsigned size;
+static void * addr;
+static unsigned buf[SIZE];
+
+void testOutOfBounds() {
+ // Not out of bounds.
+ buf[SIZE-1] = 1; // no-warning
+}
+
+void testOutOfBoundsCopy1() {
+ memcpy(buf, addr, size); // no-warning
+}
+
+void testOutOfBoundsCopy2() {
+ memcpy(addr, buf, size); // no-warning
+}
+
#endif
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1341,7 +1341,8 @@
// If a variable is reinterpreted as a type that doesn't fit into a larger
// type evenly, round it down.
// This is a signed value, since it's used in arithmetic with signed indices.
- return svalBuilder.makeIntVal(RegionSize / EleSize, false);
+ return svalBuilder.makeIntVal(RegionSize / EleSize,
+svalBuilder.getArrayIndexType());
}
//===--===//
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -336,9 +336,8 @@
// Get the offset: the minimum value of the array index type.
BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
- // FIXME: This should be using ValueManager::ArrayindexTy...somehow.
if (indexTy.isNull())
-indexTy = Ctx.IntTy;
+indexTy = svalBuilder.getArrayIndexType();
nonloc::ConcreteInt Min(BVF.getMinValue(indexTy));
// Adjust the index.
Index: test/Analysis/index-type.c
===
--- test/Analysis/index-type.c
+++ test/Analysis/index-type.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -DM32 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -verify %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -DM32 -verify %s
// expected-no-diagnostics
#define UINT_MAX (~0u)
@@ -36,4 +36,23 @@
*ptr = 42; // no-warning
}
+#define SIZE 4294967296
+
+static unsigned size;
+static void * addr;
+static unsigned buf[SIZE];
+
+void testOutOfBounds() {
+ // Not out of bounds.
+ buf[SIZE-1] = 1; // no-warning
+}
+
+void testOutOfBoundsCopy1() {
+ memcpy(buf, addr, size); // no-warning
+}
+
+void testOutOfBoundsCopy2() {
+ memcpy(addr, buf, size); // no-warning
+}
+
#endif
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1341,7 +1341,8 @@
// If a variable is reinterpreted as a type that doesn't fit into a larger
// type evenly, round it down.
// This is a signed value, since it's used in arithmetic with signed indices.
- return svalBuilder.makeIntVal(RegionSize / EleSize, false);
+ return svalBuilder.makeIntVal(RegionSize / EleSize,
+svalBuilder.getArrayIndexType());
}
//===--===//
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -336,9 +336,8 @@
// Get the offset: the minim
[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR
RKSimon requested changes to this revision. RKSimon added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D41168#1118624, @tkrupa wrote: > Mask scalar case is closed and doesn't have any effects on this revision. > Besides, I resolved issues connected to lowering scalar sqrt intrinsics > without rounding (that is, if https://reviews.llvm.org/D47621 is accepted). > Should I add them here to have everything sqrt in one place or upstream this > and add them to a new revision? Add them here please Repository: rC Clang https://reviews.llvm.org/D41168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47460: [Frontend] Honor UserFilesAreVolatile flag getting file buffer in ASTUnit
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Is it plausible to add a unit-test for this? https://reviews.llvm.org/D47460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333734 - [X86] Remove leftover semicolons at end of macros
Author: mstorsjo
Date: Fri Jun 1 02:40:50 2018
New Revision: 333734
URL: http://llvm.org/viewvc/llvm-project?rev=333734&view=rev
Log:
[X86] Remove leftover semicolons at end of macros
This was missed in a few places in SVN r333613, causing compilation
errors if these macros are used e.g. as parameter to a function.
Modified:
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/f16cintrin.h
cfe/trunk/lib/Headers/gfniintrin.h
cfe/trunk/lib/Headers/shaintrin.h
cfe/trunk/lib/Headers/vpclmulqdqintrin.h
Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=333734&r1=333733&r2=333734&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Fri Jun 1 02:40:50 2018
@@ -2226,13 +2226,13 @@ _mm512_maskz_sub_ps(__mmask16 __U, __m51
(__m512)__builtin_ia32_subps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)(__m512)(W), (__mmask16)(U), \
- (int)(R));
+ (int)(R))
#define _mm512_maskz_sub_round_ps(U, A, B, R) \
(__m512)__builtin_ia32_subps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)_mm512_setzero_ps(), \
- (__mmask16)(U), (int)(R));
+ (__mmask16)(U), (int)(R))
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_mask_mul_ss(__m128 __W, __mmask8 __U,__m128 __A, __m128 __B) {
@@ -2361,13 +2361,13 @@ _mm512_maskz_mul_ps(__mmask16 __U, __m51
(__m512)__builtin_ia32_mulps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)(__m512)(W), (__mmask16)(U), \
- (int)(R));
+ (int)(R))
#define _mm512_maskz_mul_round_ps(U, A, B, R) \
(__m512)__builtin_ia32_mulps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)_mm512_setzero_ps(), \
- (__mmask16)(U), (int)(R));
+ (__mmask16)(U), (int)(R))
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_mask_div_ss(__m128 __W, __mmask8 __U,__m128 __A, __m128 __B) {
@@ -2509,13 +2509,13 @@ _mm512_maskz_div_ps(__mmask16 __U, __m51
(__m512)__builtin_ia32_divps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)(__m512)(W), (__mmask16)(U), \
- (int)(R));
+ (int)(R))
#define _mm512_maskz_div_round_ps(U, A, B, R) \
(__m512)__builtin_ia32_divps512_mask((__v16sf)(__m512)(A), \
(__v16sf)(__m512)(B), \
(__v16sf)_mm512_setzero_ps(), \
- (__mmask16)(U), (int)(R));
+ (__mmask16)(U), (int)(R))
#define _mm512_roundscale_ps(A, B) \
(__m512)__builtin_ia32_rndscaleps_mask((__v16sf)(__m512)(A), (int)(B), \
Modified: cfe/trunk/lib/Headers/f16cintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/f16cintrin.h?rev=333734&r1=333733&r2=333734&view=diff
==
--- cfe/trunk/lib/Headers/f16cintrin.h (original)
+++ cfe/trunk/lib/Headers/f16cintrin.h Fri Jun 1 02:40:50 2018
@@ -79,7 +79,7 @@ _cvtsh_ss(unsigned short __a)
/// \returns The converted 16-bit half-precision float value.
#define _cvtss_sh(a, imm) \
(unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
- (imm)))[0]);
+ (imm)))[0])
/// Converts a 128-bit vector containing 32-bit float values into a
///128-bit vector containing 16-bit half-precision float values.
@@ -105,7 +105,7 @@ _cvtsh_ss(unsigned short __a)
///values. The lower 64 bits are used to store the converted 16-bit
///half-precision floating-point values.
#define _mm_cvtps_ph(a, imm) \
- (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm));
+ (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm))
/// Converts a 128-bit vector containing 16-bit half-precision float
///values into a 128-bit vector containing 32-bit float values.
@@ -148,7 +148,7 @@ _mm_cvtph_ps(__m128i __a)
/// \returns A 128-bit vector containing the converted 16-bit half-precision
///float values.
#def
[PATCH] D46892: [X86] Lowering adds/addus/subs/subus intrinsics to native IR (Clang part)
tkrupa updated this revision to Diff 149417.
tkrupa added a comment.
Added missing scalar intrinsics without rounding.
Repository:
rC Clang
https://reviews.llvm.org/D46892
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
test/CodeGen/avx512vl-builtins.c
test/CodeGen/sse-builtins.c
test/CodeGen/sse2-builtins.c
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -1190,17 +1190,16 @@
__m128d test_mm_sqrt_pd(__m128d A) {
// CHECK-LABEL: test_mm_sqrt_pd
- // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.pd(<2 x double> %{{.*}})
+ // CHECK: call <2 x double> @llvm.sqrt.v2f64(<2 x double> %{{.*}})
return _mm_sqrt_pd(A);
}
__m128d test_mm_sqrt_sd(__m128d A, __m128d B) {
// CHECK-LABEL: test_mm_sqrt_sd
- // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.sd(<2 x double> %{{.*}})
- // CHECK: extractelement <2 x double> %{{.*}}, i32 0
- // CHECK: insertelement <2 x double> undef, double %{{.*}}, i32 0
- // CHECK: extractelement <2 x double> %{{.*}}, i32 1
- // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i32 1
+ // CHECK-NOT: call <2 x double> @llvm.x86.sse2.sqrt.sd(<2 x double> %{{.*}})
+ // CHECK: extractelement <2 x double> %{{.*}}, i64 0
+ // CHECK: call double @llvm.sqrt.f64(double {{.*}})
+ // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i64 0
return _mm_sqrt_sd(A, B);
}
Index: test/CodeGen/sse-builtins.c
===
--- test/CodeGen/sse-builtins.c
+++ test/CodeGen/sse-builtins.c
@@ -639,13 +639,16 @@
__m128 test_mm_sqrt_ps(__m128 x) {
// CHECK-LABEL: test_mm_sqrt_ps
- // CHECK: call <4 x float> @llvm.x86.sse.sqrt.ps(<4 x float> {{.*}})
+ // CHECK: call <4 x float> @llvm.sqrt.v4f32(<4 x float> {{.*}})
return _mm_sqrt_ps(x);
}
__m128 test_sqrt_ss(__m128 x) {
// CHECK: define {{.*}} @test_sqrt_ss
- // CHECK: call <4 x float> @llvm.x86.sse.sqrt.ss
+ // CHECK-NOT: call <4 x float> @llvm.x86.sse.sqrt.ss
+ // CHECK: extractelement <4 x float> {{.*}}, i64 0
+ // CHECK: call float @llvm.sqrt.f32(float {{.*}})
+ // CHECK: insertelement <4 x float> {{.*}}, float {{.*}}, i64 0
return _mm_sqrt_ss(x);
}
Index: test/CodeGen/avx512vl-builtins.c
===
--- test/CodeGen/avx512vl-builtins.c
+++ test/CodeGen/avx512vl-builtins.c
@@ -3506,49 +3506,49 @@
}
__m128d test_mm_mask_sqrt_pd(__m128d __W, __mmask8 __U, __m128d __A) {
// CHECK-LABEL: @test_mm_mask_sqrt_pd
- // CHECK: @llvm.x86.sse2.sqrt.pd
+ // CHECK: @llvm.sqrt.v2f64
// CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
return _mm_mask_sqrt_pd(__W,__U,__A);
}
__m128d test_mm_maskz_sqrt_pd(__mmask8 __U, __m128d __A) {
// CHECK-LABEL: @test_mm_maskz_sqrt_pd
- // CHECK: @llvm.x86.sse2.sqrt.pd
+ // CHECK: @llvm.sqrt.v2f64
// CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
return _mm_maskz_sqrt_pd(__U,__A);
}
__m256d test_mm256_mask_sqrt_pd(__m256d __W, __mmask8 __U, __m256d __A) {
// CHECK-LABEL: @test_mm256_mask_sqrt_pd
- // CHECK: @llvm.x86.avx.sqrt.pd.256
+ // CHECK: @llvm.sqrt.v4f64
// CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
return _mm256_mask_sqrt_pd(__W,__U,__A);
}
__m256d test_mm256_maskz_sqrt_pd(__mmask8 __U, __m256d __A) {
// CHECK-LABEL: @test_mm256_maskz_sqrt_pd
- // CHECK: @llvm.x86.avx.sqrt.pd.256
+ // CHECK: @llvm.sqrt.v4f64
// CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
return _mm256_maskz_sqrt_pd(__U,__A);
}
__m128 test_mm_mask_sqrt_ps(__m128 __W, __mmask8 __U, __m128 __A) {
// CHECK-LABEL: @test_mm_mask_sqrt_ps
- // CHECK: @llvm.x86.sse.sqrt.ps
+ // CHECK: @llvm.sqrt.v4f32
// CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
return _mm_mask_sqrt_ps(__W,__U,__A);
}
__m128 test_mm_maskz_sqrt_ps(__mmask8 __U, __m128 __A) {
// CHECK-LABEL: @test_mm_maskz_sqrt_ps
- // CHECK: @llvm.x86.sse.sqrt.ps
+ // CHECK: @llvm.sqrt.v4f32
// CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
return _mm_maskz_sqrt_ps(__U,__A);
}
__m256 test_mm256_mask_sqrt_ps(__m256 __W, __mmask8 __U, __m256 __A) {
// CHECK-LABEL: @test_mm256_mask_sqrt_ps
- // CHECK: @llvm.x86.avx.sqrt.ps.256
+ // CHECK: @llvm.sqrt.v8f32
// CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
return _mm256_mask_sqrt_ps(__W,__U,__A);
}
__m256 test_mm256_maskz_sqrt_ps(__mmask8 __U, __m256 __A) {
// CHECK-LABEL: @test_mm256_maskz_sqrt_ps
- // CHECK: @llvm.x86.avx.sqrt.ps.256
+ // CHECK: @llvm.sqrt.v8f32
// CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
return _mm256_maskz_sqrt_ps(__U,__A);
}
Index: test/CodeGen/avx512f-bu
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov, klimek.
These decls are sometime used as the canonical declarations (e.g. for
go-to-def),
which seems to be bad.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,29 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]() {}
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,15 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
+ return true;
+D = ASTNode.OrigD;
+ }
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,29 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]() {}
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,15 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
+ return true;
+D = ASTNode.OrigD;
+ }
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR
tkrupa updated this revision to Diff 149419.
tkrupa added a comment.
Added missing scalar intrinsics without rounding.
Repository:
rC Clang
https://reviews.llvm.org/D41168
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx-builtins.c
test/CodeGen/avx512f-builtins.c
test/CodeGen/avx512vl-builtins.c
test/CodeGen/sse-builtins.c
test/CodeGen/sse2-builtins.c
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -1190,17 +1190,16 @@
__m128d test_mm_sqrt_pd(__m128d A) {
// CHECK-LABEL: test_mm_sqrt_pd
- // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.pd(<2 x double> %{{.*}})
+ // CHECK: call <2 x double> @llvm.sqrt.v2f64(<2 x double> %{{.*}})
return _mm_sqrt_pd(A);
}
__m128d test_mm_sqrt_sd(__m128d A, __m128d B) {
// CHECK-LABEL: test_mm_sqrt_sd
- // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.sd(<2 x double> %{{.*}})
- // CHECK: extractelement <2 x double> %{{.*}}, i32 0
- // CHECK: insertelement <2 x double> undef, double %{{.*}}, i32 0
- // CHECK: extractelement <2 x double> %{{.*}}, i32 1
- // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i32 1
+ // CHECK-NOT: call <2 x double> @llvm.x86.sse2.sqrt.sd(<2 x double> %{{.*}})
+ // CHECK: extractelement <2 x double> %{{.*}}, i64 0
+ // CHECK: call double @llvm.sqrt.f64(double {{.*}})
+ // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i64 0
return _mm_sqrt_sd(A, B);
}
Index: test/CodeGen/sse-builtins.c
===
--- test/CodeGen/sse-builtins.c
+++ test/CodeGen/sse-builtins.c
@@ -639,13 +639,16 @@
__m128 test_mm_sqrt_ps(__m128 x) {
// CHECK-LABEL: test_mm_sqrt_ps
- // CHECK: call <4 x float> @llvm.x86.sse.sqrt.ps(<4 x float> {{.*}})
+ // CHECK: call <4 x float> @llvm.sqrt.v4f32(<4 x float> {{.*}})
return _mm_sqrt_ps(x);
}
__m128 test_sqrt_ss(__m128 x) {
// CHECK: define {{.*}} @test_sqrt_ss
- // CHECK: call <4 x float> @llvm.x86.sse.sqrt.ss
+ // CHECK-NOT: call <4 x float> @llvm.x86.sse.sqrt.ss
+ // CHECK: extractelement <4 x float> {{.*}}, i64 0
+ // CHECK: call float @llvm.sqrt.f32(float {{.*}})
+ // CHECK: insertelement <4 x float> {{.*}}, float {{.*}}, i64 0
return _mm_sqrt_ss(x);
}
Index: test/CodeGen/avx512vl-builtins.c
===
--- test/CodeGen/avx512vl-builtins.c
+++ test/CodeGen/avx512vl-builtins.c
@@ -3506,49 +3506,49 @@
}
__m128d test_mm_mask_sqrt_pd(__m128d __W, __mmask8 __U, __m128d __A) {
// CHECK-LABEL: @test_mm_mask_sqrt_pd
- // CHECK: @llvm.x86.sse2.sqrt.pd
+ // CHECK: @llvm.sqrt.v2f64
// CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
return _mm_mask_sqrt_pd(__W,__U,__A);
}
__m128d test_mm_maskz_sqrt_pd(__mmask8 __U, __m128d __A) {
// CHECK-LABEL: @test_mm_maskz_sqrt_pd
- // CHECK: @llvm.x86.sse2.sqrt.pd
+ // CHECK: @llvm.sqrt.v2f64
// CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
return _mm_maskz_sqrt_pd(__U,__A);
}
__m256d test_mm256_mask_sqrt_pd(__m256d __W, __mmask8 __U, __m256d __A) {
// CHECK-LABEL: @test_mm256_mask_sqrt_pd
- // CHECK: @llvm.x86.avx.sqrt.pd.256
+ // CHECK: @llvm.sqrt.v4f64
// CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
return _mm256_mask_sqrt_pd(__W,__U,__A);
}
__m256d test_mm256_maskz_sqrt_pd(__mmask8 __U, __m256d __A) {
// CHECK-LABEL: @test_mm256_maskz_sqrt_pd
- // CHECK: @llvm.x86.avx.sqrt.pd.256
+ // CHECK: @llvm.sqrt.v4f64
// CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
return _mm256_maskz_sqrt_pd(__U,__A);
}
__m128 test_mm_mask_sqrt_ps(__m128 __W, __mmask8 __U, __m128 __A) {
// CHECK-LABEL: @test_mm_mask_sqrt_ps
- // CHECK: @llvm.x86.sse.sqrt.ps
+ // CHECK: @llvm.sqrt.v4f32
// CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
return _mm_mask_sqrt_ps(__W,__U,__A);
}
__m128 test_mm_maskz_sqrt_ps(__mmask8 __U, __m128 __A) {
// CHECK-LABEL: @test_mm_maskz_sqrt_ps
- // CHECK: @llvm.x86.sse.sqrt.ps
+ // CHECK: @llvm.sqrt.v4f32
// CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
return _mm_maskz_sqrt_ps(__U,__A);
}
__m256 test_mm256_mask_sqrt_ps(__m256 __W, __mmask8 __U, __m256 __A) {
// CHECK-LABEL: @test_mm256_mask_sqrt_ps
- // CHECK: @llvm.x86.avx.sqrt.ps.256
+ // CHECK: @llvm.sqrt.v8f32
// CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
return _mm256_mask_sqrt_ps(__W,__U,__A);
}
__m256 test_mm256_maskz_sqrt_ps(__mmask8 __U, __m256 __A) {
// CHECK-LABEL: @test_mm256_maskz_sqrt_ps
- // CHECK: @llvm.x86.avx.sqrt.ps.256
+ // CHECK: @llvm.sqrt.v8f32
// CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
return _mm256_maskz_sqrt_ps(__U,__A);
}
Index: test/CodeGen/avx512f-bu
r333735 - [CodeComplete] Add a few extra tests for r333538. NFC
Author: ibiryukov
Date: Fri Jun 1 02:49:53 2018
New Revision: 333735
URL: http://llvm.org/viewvc/llvm-project?rev=333735&view=rev
Log:
[CodeComplete] Add a few extra tests for r333538. NFC
From a follow-up discussion in D44480.
New tests check that function bodies are not skipped:
- In presence of ptr declarators, e.g. `auto**`.
- When `decltype(auto)` is used in return type, only `auto` was checked before.
Modified:
cfe/trunk/test/CodeCompletion/skip-auto-funcs.cpp
Modified: cfe/trunk/test/CodeCompletion/skip-auto-funcs.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/skip-auto-funcs.cpp?rev=333735&r1=333734&r2=333735&view=diff
==
--- cfe/trunk/test/CodeCompletion/skip-auto-funcs.cpp (original)
+++ cfe/trunk/test/CodeCompletion/skip-auto-funcs.cpp Fri Jun 1 02:49:53 2018
@@ -1,7 +1,7 @@
// We run clang in completion mode to force skipping of function bodies and
// check if the function bodies were skipped by observing the warnings that
// clang produces.
-// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:42:1 %s -o - 2>&1
| FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:60:1 %s -o - 2>&1
| FileCheck %s
template
auto not_skipped() {
int x;
@@ -37,6 +37,24 @@ auto lambda_skipped = []() -> int {
return 1;
};
+template
+decltype(auto)** not_skipped_ptr() {
+ int x;
+ if (x = 10) {}
+ // Check that this function is not skipped.
+ // CHECK: 43:9: warning: using the result of an assignment as a condition
without parentheses
+ return T();
+}
+
+template
+decltype(auto) not_skipped_decltypeauto() {
+ int x;
+ if (x = 10) {}
+ // Check that this function is not skipped.
+ // CHECK: 52:9: warning: using the result of an assignment as a condition
without parentheses
+ return 1;
+}
+
int test() {
int complete_in_this_function;
// CHECK: COMPLETION: complete_in_this_function
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46892: [X86] Lowering adds/addus/subs/subus intrinsics to native IR (Clang part)
tkrupa added a comment. Whoops, that's a wrong revision. I'll revert it shortly. Repository: rC Clang https://reviews.llvm.org/D46892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46892: [X86] Lowering adds/addus/subs/subus intrinsics to native IR (Clang part)
tkrupa updated this revision to Diff 149420.
Repository:
rC Clang
https://reviews.llvm.org/D46892
Files:
lib/CodeGen/CGBuiltin.cpp
test/CodeGen/avx2-builtins.c
test/CodeGen/avx512bw-builtins.c
test/CodeGen/avx512vlbw-builtins.c
test/CodeGen/sse2-builtins.c
Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -47,25 +47,47 @@
__m128i test_mm_adds_epi8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epi8
- // CHECK: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16>
+ // CHECK: icmp slt <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+ // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
return _mm_adds_epi8(A, B);
}
__m128i test_mm_adds_epi16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epi16
- // CHECK: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32>
+ // CHECK: icmp slt <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+ // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
return _mm_adds_epi16(A, B);
}
__m128i test_mm_adds_epu8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epu8
- // CHECK: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: add <16 x i8> %{{.*}}, %{{.*}}
+ // CHECK: icmp ugt <16 x i8> %{{.*}}, %{{.*}}
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i8> , <16 x i8> {{.*}}
return _mm_adds_epu8(A, B);
}
__m128i test_mm_adds_epu16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_adds_epu16
- // CHECK: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: add <8 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp ugt <8 x i16> %{{.*}}, %{{.*}}
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i16> , <8 x i16> {{.*}}
return _mm_adds_epu16(A, B);
}
@@ -1416,25 +1438,47 @@
__m128i test_mm_subs_epi8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epi8
- // CHECK: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+ // CHECK: sub <16 x i16> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16>
+ // CHECK: icmp slt <16 x i16> %{{.*}},
+ // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+ // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
return _mm_subs_epi8(A, B);
}
__m128i test_mm_subs_epi16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epi16
- // CHECK: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+ // CHECK: sub <8 x i32> %{{.*}}, %{{.*}}
+ // CHECK: icmp sle <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32>
+ // CHECK: icmp slt <8 x i32> %{{.*}},
+ // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+ // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
return _mm_subs_epi16(A, B);
}
__m128i test_mm_subs_epu8(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epu8
- // CHECK: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+ // CHECK: icmp ugt <16 x i8> {{.*}}, {{.*}}
+ // CHECK: select <16 x i1> {{.*}}, <16 x i8> {{.*}}, <16 x i8> {{.*}}
+ // CHECK: sub <16 x i8> {{.*}}, {{.*}}
return _mm_subs_epu8(A, B);
}
__m128i test_mm_subs_epu16(__m128i A, __m128i B) {
// CHECK-LABEL: test_mm_subs_epu16
- // CHECK: call <8 x i16> @llvm.x86.sse2.psubus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+ // CHECK-NO
[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type
ilya-biryukov added a comment. In https://reviews.llvm.org/D44480#1117230, @nik wrote: > In https://reviews.llvm.org/D44480#1117147, @cpplearner wrote: > > > Does `getAs()` work correctly with function returning `auto&`? > > > the "getAs()" version will skip the function body and generate an > error message on use, but "FD->getReturnType()->getContainedDeducedType()" > works fine (will not skip the body, as it should here). OK, so now I have a > rough idea what the "Contained" was referring too :) Yep, `getContainedDeducedType()` call is definitely necessary. Added the tests for discussed cases in https://reviews.llvm.org/rL333735. Repository: rL LLVM https://reviews.llvm.org/D44480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333737 - [clangd] Keep only a limited number of idle ASTs in memory
Author: ibiryukov
Date: Fri Jun 1 03:08:43 2018
New Revision: 333737
URL: http://llvm.org/viewvc/llvm-project?rev=333737&view=rev
Log:
[clangd] Keep only a limited number of idle ASTs in memory
Summary:
After this commit, clangd will only keep the last 3 accessed ASTs in
memory. Preambles for each of the opened files are still kept in
memory to make completion and AST rebuilds fast.
AST rebuilds are usually fast enough, but having the last ASTs in
memory still considerably improves latency of operations like
findDefinition and documeneHighlight, which are often sent multiple
times a second when moving around the code. So keeping some of the last
accessed ASTs in memory seems like a reasonable tradeoff.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: malaperle, arphaman, klimek, javed.absar, ioeric, MaskRay,
jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47063
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/test/clangd/trace.test
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=333737&r1=333736&r2=333737&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Jun 1 03:08:43 2018
@@ -100,7 +100,7 @@ ClangdServer::ClangdServer(GlobalCompila
std::shared_ptr
PP) { FileIdx->update(Path, &AST, std::move(PP)); }
: PreambleParsedCallback(),
- Opts.UpdateDebounce) {
+ Opts.UpdateDebounce, Opts.RetentionPolicy) {
if (FileIdx && Opts.StaticIndex) {
MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
Index = MergedIndex.get();
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=333737&r1=333736&r2=333737&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Jun 1 03:08:43 2018
@@ -70,6 +70,9 @@ public:
/// If 0, all requests are processed on the calling thread.
unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
+/// AST caching policy. The default is to keep up to 3 ASTs in memory.
+ASTRetentionPolicy RetentionPolicy;
+
/// Cached preambles are potentially large. If false, store them on disk.
bool StorePreamblesInMemory = true;
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333737&r1=333736&r2=333737&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Jun 1 03:08:43 2018
@@ -175,8 +175,12 @@ ParsedAST::Build(std::unique_ptr ParsedDecls = Action->takeTopLevelDecls();
+ std::vector Diags = ASTDiags.take();
+ // Add diagnostics from the preamble, if any.
+ if (Preamble)
+Diags.insert(Diags.begin(), Preamble->Diags.begin(),
Preamble->Diags.end());
return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
- std::move(ParsedDecls), ASTDiags.take(),
+ std::move(ParsedDecls), std::move(Diags),
std::move(Inclusions));
}
@@ -243,120 +247,57 @@ ParsedAST::ParsedAST(std::shared_ptrAction);
}
-CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
- std::shared_ptr PCHs,
- PreambleParsedCallback PreambleCallback)
-: FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
- PCHs(std::move(PCHs)), PreambleCallback(std::move(PreambleCallback)) {
- log("Created CppFile for " + FileName);
-}
-
-llvm::Optional> CppFile::rebuild(ParseInputs &&Inputs) {
- log("Rebuilding file " + FileName + " with command [" +
- Inputs.CompileCommand.Directory + "] " +
- llvm::join(Inputs.CompileCommand.CommandLine, " "));
-
+std::unique_ptr
+clangd::buildCompilerInvocation(const ParseInputs &Inputs) {
std::vector ArgStrs;
for (const auto &S : Inputs.CompileCommand.CommandLine)
ArgStrs.push_back(S.c_str());
if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
-log("Couldn't set working directory");
-// We
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov added inline comments.
Comment at: clangd/TUScheduler.h:66
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionPolicy RetentionPolicy = {});
~TUScheduler();
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > does this actually have more than one caller? what's the plan for
> > > exposing this option to embedders/CLI users (not saying we necessarily
> > > need the latter)?
> > Yes, just one caller outside the tests.
> > The plan was to expose it only in `ClangdServer` for now. Giving this knob
> > in CLI might be useful, if we have good reasons for that, but I hope that
> > we could pick the default that work for everyone instead.
> > Added that as a parameter of `ClangdServer`.
> >
> > Maybe we should move the default value of 3 to `ClangdServer`? WDYT?
> CLI when needed SG.
> I think we want to give our cloud folks the chance to set it to zero.
> So maybe put the default in ClangdLSPServer? (or make it a param there too
> and move the value to main)
After an online chat we've decided to keep the default inside the
ASTRetentionPolicy struct.
If we add the CLI parameter, we can pass custom values from main to
ClangdLSPServer.
And other clients already have a knob to set it in ClangdServer.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333737: [clangd] Keep only a limited number of idle ASTs in
memory (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47063
Files:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/test/clangd/trace.test
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
@@ -18,8 +18,11 @@
namespace clang {
namespace clangd {
+using ::testing::_;
+using ::testing::AnyOf;
using ::testing::Pair;
using ::testing::Pointee;
+using ::testing::UnorderedElementsAre;
void ignoreUpdate(llvm::Optional>) {}
void ignoreError(llvm::Error Err) {
@@ -43,7 +46,8 @@
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
/*PreambleParsedCallback=*/nullptr,
-/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ASTRetentionPolicy());
auto Added = testPath("added.cpp");
Files[Added] = "";
@@ -99,7 +103,8 @@
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
/*PreambleParsedCallback=*/nullptr,
-/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ASTRetentionPolicy());
auto Path = testPath("foo.cpp");
S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
[&](std::vector) { Ready.wait(); });
@@ -127,7 +132,8 @@
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
/*PreambleParsedCallback=*/nullptr,
- /*UpdateDebounce=*/std::chrono::seconds(1));
+ /*UpdateDebounce=*/std::chrono::seconds(1),
+ ASTRetentionPolicy());
// FIXME: we could probably use timeouts lower than 1 second here.
auto Path = testPath("foo.cpp");
S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
@@ -158,7 +164,8 @@
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
/*PreambleParsedCallback=*/nullptr,
- /*UpdateDebounce=*/std::chrono::milliseconds(50));
+ /*UpdateDebounce=*/std::chrono::milliseconds(50),
+ ASTRetentionPolicy());
std::vector Files;
for (int I = 0; I < FilesCount; ++I) {
@@ -219,18 +226,18 @@
{
WithContextValue WithNonce(NonceKey, ++Nonce);
- S.runWithPreamble(
- "CheckPreamble", File,
- [Inputs, Nonce, &Mut, &TotalPreambleReads](
- llvm::Expected Preamble) {
-EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
-
-ASSERT_TRUE((bool)Preamble);
-EXPECT_EQ(Preamble->Contents, Inputs.Contents);
-
-std::lock_guard Lock(Mut);
-++TotalPreambleReads;
- });
+ S.runWithPreamble("CheckPreamble", File,
+[Inputs, Nonce, &Mut, &TotalPreambleReads](
+llvm::Expected Preamble) {
+ EXPECT_THAT(Context::current().get(NonceKey),
+ Pointee(Nonce));
+
+ ASSERT_TRUE((bool)Preamble);
+ EXPECT_EQ(Preamble->Contents, Inputs.Contents);
+
+ std::lock_guard Lock(Mut);
+ ++TotalPreambleReads;
+});
}
}
}
@@ -242,5 +249,55 @@
EXPECT_EQ(TotalPreambleReads, FilesCount * UpdatesPerFile);
}
+TEST_F(TUSchedulerTests, EvictedAST) {
+ ASTRetentionPolicy Policy;
+ Policy.MaxRetainedASTs = 2;
+ TUScheduler S(
+ /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
+ PreambleParsedCallback(),
+ /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
+
+ llvm::StringLiteral SourceContents = R"cpp(
+int* a;
+double* b = a;
+ )cpp";
+
+ auto Foo = testPath("foo.cpp");
+ auto Bar = testPath("bar.cpp");
+ auto Baz = testPath("baz.cpp");
+
+ std::atomic BuiltASTC
[PATCH] D47460: [Frontend] Honor UserFilesAreVolatile flag getting file buffer in ASTUnit
yvvan added a comment. In https://reviews.llvm.org/D47460#1118694, @ilya-biryukov wrote: > Is it plausible to add a unit-test for this? i think I can add a unit-test for it since we have the 'getBufferKind' method in MemoryBuffer. https://reviews.llvm.org/D47460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso added inline comments. Comment at: docs/ReleaseNotes.rst:139 + used in conjunction with ``-Werror`` and as a result, the new warnings + are turned into new errors. + thakis wrote: > nit: I'd omit this paragraph -- this is true for all warnings and not special > for this warning. Removed that specific paragraph. Comment at: include/clang/Basic/DiagnosticGroups.td:828-829 // -Wunused-local-typedefs = -Wunused-local-typedef +def : DiagGroup<"unused-usings", [UnusedUsing]>; +// -Wunused-usings = -Wunused-using thakis wrote: > CarlosAlbertoEnciso wrote: > > lebedev.ri wrote: > > > Why? gcc compatibility? > > No particular reason. I just follow the 'unused-local-typedefs' model. > > If there is not objection from others reviewers, I will drop the gcc > > compatibility. > Does gcc have a `-Wunused-usings`? As far as I can tell it doesn't, so I > agree not having the alias makes sense. -Wunused-local-typedefs is here > because gcc has that flag. As far as I can see, GCC does not have the ``-Wunused-usings`` alias. https://reviews.llvm.org/D44826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso updated this revision to Diff 149425.
CarlosAlbertoEnciso marked 3 inline comments as done.
CarlosAlbertoEnciso added a comment.
Address feedback from @thakis in relation to the Release Notes.
https://reviews.llvm.org/D44826
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/ExternalSemaSource.h
include/clang/Sema/MultiplexExternalSemaSource.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
lib/Sema/MultiplexExternalSemaSource.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/FixIt/fixit.cpp
test/Modules/Inputs/module.map
test/Modules/Inputs/warn-unused-using.h
test/Modules/warn-unused-using.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/coreturn.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
test/SemaCXX/referenced_using_options.cpp
Index: test/SemaCXX/referenced_using_options.cpp
===
--- test/SemaCXX/referenced_using_options.cpp
+++ test/SemaCXX/referenced_using_options.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ typedef int Integer;
+}
+
+using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-using"
+using N::Integer; // no warning
+#pragma clang diagnostic pop
+
+using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ typedef int Integer;
+ int var;
+}
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N; // expected-warning {{unused using directive 'N'}}
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N; // expected-warning {{unused using directive 'N'}}
+ N::Integer var = 1;
+}
+
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ typedef int Integer;
+ typedef char Char;
+}
+
+using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+using N::Char; // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;// expected-warning {{unused using declaration 'Char'}}
+ return 0;
+}
+
Index: test/SemaCXX/referenced_using_declaration_1.cpp
===
--- test/SemaCXX/referenced_using_declaration_1.cpp
+++ test/SemaCXX/referenced_using_declaration_1.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ // Types.
+ typedef int Integer;
+ struct Record {
+int a;
+ };
+
+ // Variables.
+ int var1;
+ int var2;
+
+ // Functions.
+ void func1();
+ void func2();
+}
+
+using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+using N::Record;// expected-warning {{unused using declaration 'Record'}}
+using N::var1; // expected-warning {{unused using declaration 'var1'}}
+using N::var2; // expected-warning {{unused using declaration 'var2'}}
+using N::func1; // expected-warning {{unused using declaration 'func1'}}
+using N::func2; // expected-warning {{unused using declaration 'func2'}}
+
+void Foo() {
+ using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+ N::Integer int_var;
+ int_var = 1;
+
+ using N::Record; // Referenced
+ Record rec_var;
+ rec_var.a = 2;
+
+ using N::var1;// expected-warning {{unused using declaration 'var1'}}
+ N::var1 = 3;
+
+ using N::var2;// Referenced
+ var2 = 4;
+
+ using N::func1; // expected-warning {{unused using declaration 'func1'}}
+ N::func1();
+
+ using N::func2;
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso added a comment. Thanks to all reviewers for your comments and suggestions. https://reviews.llvm.org/D44826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
lebedev.ri added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:282-290
+def warn_unused_using_declaration : Warning<
+ "unused using declaration %0">,
+ InGroup, DefaultIgnore;
+def warn_unused_using_directive : Warning<
+ "unused using directive %0">,
+ InGroup, DefaultIgnore;
+def warn_unused_using_alias : Warning<
JFYI you can condense it down to just
```
def warn_unused_using_declaration : Warning<
"unused %select{using declaration|using directive|namespace alias}0 %1">,
InGroup, DefaultIgnore;
```
if that simplifies the code that actually emits that warning.
https://reviews.llvm.org/D44826
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D45517: [analyzer] False positive refutation with Z3
Hi, > Just a bit of context and to have some expectation management regarding > this patch. The main purpose of this implementation was to back a thesis. > It was made under a very serious time pressure and the main goal was to be > able to measure on real world projects as soon as possible and in the > meantime to be flexible so we can measure multiple configurations (like > incremental solving). > > So the goal was a flexible proof of concept that is sensible to measure in > the shortest possible time. After the thesis was done, Reka started to work > an another GSoC project, so she had no time to review the code with the > requirements of upstreaming in mind. Nevertheless we found that sharing the > proof of concept could be useful for the community. So it is perfectly > reasonable if you disagree with some design decisions behind this patch, > because the requirements for the thesis (in the short time frame) was very > different from the requirements of upstreaming this work. In a different > context these decisions made perfect sense. > > Just want to comment here and give thanks again for the first version of the refutation code. It's being really helpful to develop the approach this code as a base; things would definitely be slower if I had to start it from scratch. Thanks! -- Mikhail Ramalho. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333742 - [clangd] Attempt the fix the buildbots after r333737
Author: ibiryukov Date: Fri Jun 1 05:03:16 2018 New Revision: 333742 URL: http://llvm.org/viewvc/llvm-project?rev=333742&view=rev Log: [clangd] Attempt the fix the buildbots after r333737 Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=333742&r1=333741&r2=333742&view=diff == --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Fri Jun 1 05:03:16 2018 @@ -110,7 +110,10 @@ public: return llvm::None; std::unique_ptr V = std::move(Existing->second); LRU.erase(Existing); -return V; +// GCC 4.8 fails to compile `return V;`, as it tries to call the copy +// constructor of unique_ptr, so we call the move ctor explicitly to avoid +// this miscompile. +return llvm::Optional>(std::move(V)); } private: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r333737 - [clangd] Keep only a limited number of idle ASTs in memory
This broke buildbots. Sorry about that.
r333742 should fix them.
On Fri, Jun 1, 2018 at 12:12 PM Ilya Biryukov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: ibiryukov
> Date: Fri Jun 1 03:08:43 2018
> New Revision: 333737
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333737&view=rev
> Log:
> [clangd] Keep only a limited number of idle ASTs in memory
>
> Summary:
> After this commit, clangd will only keep the last 3 accessed ASTs in
> memory. Preambles for each of the opened files are still kept in
> memory to make completion and AST rebuilds fast.
>
> AST rebuilds are usually fast enough, but having the last ASTs in
> memory still considerably improves latency of operations like
> findDefinition and documeneHighlight, which are often sent multiple
> times a second when moving around the code. So keeping some of the last
> accessed ASTs in memory seems like a reasonable tradeoff.
>
> Reviewers: sammccall
>
> Reviewed By: sammccall
>
> Subscribers: malaperle, arphaman, klimek, javed.absar, ioeric, MaskRay,
> jkorous, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D47063
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.h
> clang-tools-extra/trunk/clangd/ClangdUnit.cpp
> clang-tools-extra/trunk/clangd/ClangdUnit.h
> clang-tools-extra/trunk/clangd/TUScheduler.cpp
> clang-tools-extra/trunk/clangd/TUScheduler.h
> clang-tools-extra/trunk/test/clangd/trace.test
> clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
> clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=333737&r1=333736&r2=333737&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Jun 1 03:08:43
> 2018
> @@ -100,7 +100,7 @@ ClangdServer::ClangdServer(GlobalCompila
> std::shared_ptr
> PP) { FileIdx->update(Path, &AST,
> std::move(PP)); }
>: PreambleParsedCallback(),
> - Opts.UpdateDebounce) {
> + Opts.UpdateDebounce, Opts.RetentionPolicy) {
>if (FileIdx && Opts.StaticIndex) {
> MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
> Index = MergedIndex.get();
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=333737&r1=333736&r2=333737&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Jun 1 03:08:43 2018
> @@ -70,6 +70,9 @@ public:
> /// If 0, all requests are processed on the calling thread.
> unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
>
> +/// AST caching policy. The default is to keep up to 3 ASTs in memory.
> +ASTRetentionPolicy RetentionPolicy;
> +
> /// Cached preambles are potentially large. If false, store them on
> disk.
> bool StorePreamblesInMemory = true;
>
>
> Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333737&r1=333736&r2=333737&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Jun 1 03:08:43 2018
> @@ -175,8 +175,12 @@ ParsedAST::Build(std::unique_ptrASTDiags.EndSourceFile();
>
>std::vector ParsedDecls = Action->takeTopLevelDecls();
> + std::vector Diags = ASTDiags.take();
> + // Add diagnostics from the preamble, if any.
> + if (Preamble)
> +Diags.insert(Diags.begin(), Preamble->Diags.begin(),
> Preamble->Diags.end());
>return ParsedAST(std::move(Preamble), std::move(Clang),
> std::move(Action),
> - std::move(ParsedDecls), ASTDiags.take(),
> + std::move(ParsedDecls), std::move(Diags),
> std::move(Inclusions));
> }
>
> @@ -243,120 +247,57 @@ ParsedAST::ParsedAST(std::shared_ptrassert(this->Action);
> }
>
> -CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
> - std::shared_ptr PCHs,
> - PreambleParsedCallback PreambleCallback)
> -: FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
> - PCHs(std::move(PCHs)),
> PreambleCallback(std::move(PreambleCallback)) {
> - log("Created CppFile for " + FileName);
> -}
> -
> -llvm::Optional> CppFile::rebuild(ParseInputs &&Inputs) {
> - log("Rebuilding file " + FileName + " with command [" +
> - Inputs
[PATCH] D47460: [Frontend] Honor UserFilesAreVolatile flag getting file buffer in ASTUnit
ilya-biryukov added a comment. In https://reviews.llvm.org/D47460#1118782, @yvvan wrote: > i think I can add a unit-test for it since we have the 'getBufferKind' method > in MemoryBuffer. That sounds good. Having a regression test that fails with descriptive messages in case anyone changes this would certainly be useful. If that's not a lot of work, of course. https://reviews.llvm.org/D47460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44954: [clangd] Add "member" symbols to the index
sammccall accepted this revision. sammccall added a comment. Thanks, LG! Comment at: clangd/CodeComplete.h:86 +// For index-based completion, we only want: +// * symbols in namespaces or translation unit scopes (e.g. no class nit: want -> consider? Comment at: clangd/CodeComplete.h:94 +// lookup rules. +bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx); } // namespace clangd A little more context: "// Other symbols still appear in the index for other purposes, like workspace/symbols or textDocument/definition, but are not used for code completion" Comment at: clangd/index/SymbolCollector.h:22 +/// \brief Collect symbols from an AST. +/// It collects most symbols except: +/// - Implicit symbols nit: can you change "symbols" here to "declarations"? currently we rely a bit too heavily on the user knowing what "symbol" means Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ilya-biryukov added inline comments. Comment at: clangd/index/SymbolCollector.cpp:293 assert(CompletionAllocator && CompletionTUInfo); + // A declaration created for a friend declaration should not be used as the + // canonical declaration in the index. Maybe move this closer to `shouldFilterDecl()`? We have similar filters there. That would also mean we properly add the reference counts for friend declarations that get a normal declaration after their usage later. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
sammccall added a comment.
Do I understand the intent of this change correctly?
- friend decls that are not definitions should be ignored for indexing purposes
- this means they should never be selected as canonical decl
- if the friend decl is the only decl, then the symbol should not be indexed
if so, that makes sense to me. I think the comments could make this a little
clearer, but it's not too bad.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
this seems suspect, we're going to treat the third decl in `friend X; X; friend
X` differently from that in `X; friend X; friend X;`.
Why? i.e. why is the inner check necessary and why does it treat the original
(meaning first, I think) decl specially?
Comment at: unittests/clangd/SymbolCollectorTests.cpp:816
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
Can you also test that:
- if a friend decl (non-definition) comes first, followed by a non-friend decl
(non-definition), then the decl *is* indexed. (maybe just drop the definition
from foo, since it's otherwise the same as Y)
- if a friend decl has a definition, and there is no other declaration, then
the decl *is* indexed (and the friend decl is canonical)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
ilya-biryukov wrote:
> Maybe move this closer to `shouldFilterDecl()`? We have similar filters there.
> That would also mean we properly add the reference counts for friend
> declarations that get a normal declaration after their usage later.
I didn't put this filter there because I think it's a bit more special than
those filters in `shouldFilterDecl`. We check the `OrigD` and we could
potentially replace `D` with `OrigD`. We could change `shouldFilterDecl` to
handle that, but I'm not sure if it's worth it.
Reference counting for friend declarations is actually a bit tricky as USRs of
the generated declarations might be ambiguous.
Consider the following exmaple:
```
namespace a {
class A {};
namespace b { class B { friend class A; }; } // b
} // a
```
I would expect the generated friend decl to be `a::A`, but it's actually
`a::b::A`! So getting USR right is a bit tricky, and I think it's probably ok
to ignore references in friend decls.
For reference, `[namespace.memdef]p3`:
"If the name in a friend declaration is neither qualified nor a template-id and
the declaration is a function or an elaborated-type-specifier, the lookup to
determine whether the entity has been previously declared shall not consider
any scopes outside the innermost enclosing namespace.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47627: [ASTContext] Make getAddrSpaceQualType replace address spaces.
ebevhan created this revision.
ebevhan added reviewers: rjmccall, arichardson.
Herald added a subscriber: cfe-commits.
The documentation for getAddrSpaceQualType says: "If T already
has an address space specifier, it is silently replaced."
The function did not do this; it asserted instead. Fix it so
that it behaves according to the comment.
Repository:
rC Clang
https://reviews.llvm.org/D47627
Files:
lib/AST/ASTContext.cpp
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -2496,12 +2496,16 @@
const Type *TypeNode = Quals.strip(T);
// If this type already has an address space specified, it cannot get
- // another one.
- assert(!Quals.hasAddressSpace() &&
- "Type cannot be in multiple addr spaces!");
- Quals.addAddressSpace(AddressSpace);
+ // another one. Replace it.
+ if (Quals.hasAddressSpace())
+Quals.removeAddressSpace();
+ if (AddressSpace != LangAS::Default)
+Quals.addAddressSpace(AddressSpace);
- return getExtQualType(TypeNode, Quals);
+ if (Quals.hasNonFastQualifiers())
+return getExtQualType(TypeNode, Quals);
+ else
+return QualType(TypeNode, Quals.getFastQualifiers());
}
QualType ASTContext::removeAddrSpaceQualType(QualType T) const {
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -2496,12 +2496,16 @@
const Type *TypeNode = Quals.strip(T);
// If this type already has an address space specified, it cannot get
- // another one.
- assert(!Quals.hasAddressSpace() &&
- "Type cannot be in multiple addr spaces!");
- Quals.addAddressSpace(AddressSpace);
+ // another one. Replace it.
+ if (Quals.hasAddressSpace())
+Quals.removeAddressSpace();
+ if (AddressSpace != LangAS::Default)
+Quals.addAddressSpace(AddressSpace);
- return getExtQualType(TypeNode, Quals);
+ if (Quals.hasNonFastQualifiers())
+return getExtQualType(TypeNode, Quals);
+ else
+return QualType(TypeNode, Quals.getFastQualifiers());
}
QualType ASTContext::removeAddrSpaceQualType(QualType T) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47628: Detect an incompatible VLA pointer assignment
jmorse created this revision.
jmorse added reviewers: eli.friedman, majnemer.
Herald added a subscriber: cfe-commits.
For pointer assignments of VLA types, Clang currently detects when array
dimensions _lower_ than a variable dimension differ, and reports a warning.
However it does not do the same when the _higher_ dimensions differ, a
case that GCC does catch.
These two pointer types
int (*foo)[1][bar][3];
int (*baz)[1][2][3];
are compatible with each another, and the program is well formed if
bar == 2, a matter that is the programmers problem. However the following:
int (*qux)[2][2][3];
would not be compatible with either, because the upper dimension differs
in size. Clang reports baz is incompatible with qux, but not that foo is
incompatible with qux because it doesn't check those higher dimensions.
Fix this by comparing array sizes on higher dimensions: if both are
constants but unequal then report incompatibility; if either dimension is
variable then we can't know either way.
Repository:
rC Clang
https://reviews.llvm.org/D47628
Files:
lib/AST/ASTContext.cpp
test/Sema/vla.c
Index: test/Sema/vla.c
===
--- test/Sema/vla.c
+++ test/Sema/vla.c
@@ -76,3 +76,16 @@
];
};
int (*use_implicitly_declared)() = implicitly_declared; // ok, was implicitly
declared at file scope
+
+void VLAPtrAssign(int size) {
+ int array[1][2][3][size][4][5];
+ // This is well formed
+ int (*p)[2][3][size][4][5] = array;
+ // Last array dimension too large
+ int (*p2)[2][3][size][4][6] = array; // expected-warning {{incompatible
pointer types}}
+ // Second array dimension too large
+ int (*p3)[20][3][size][4][5] = array; // expected-warning {{incompatible
pointer types}}
+
+ // Not illegal in C, program _might_ be well formed if size == 3.
+ int (*p4)[2][size][3][4][5] = array;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -8572,16 +8572,46 @@
QualType ResultType = mergeTypes(LHSElem, RHSElem, false, Unqualified);
if (ResultType.isNull())
return {};
+
+const VariableArrayType* LVAT = getAsVariableArrayType(LHS);
+const VariableArrayType* RVAT = getAsVariableArrayType(RHS);
+
+// If either side is a variable array, and both are complete, check whether
+// the current dimension is definite.
+if (LVAT || RVAT) {
+ auto SizeFetch = [this](const VariableArrayType* VAT,
+ const ConstantArrayType* CAT)
+ -> std::pair {
+if (VAT) {
+ llvm::APSInt TheInt;
+ Expr *E = VAT->getSizeExpr();
+ if (E && VAT->getSizeExpr()->isIntegerConstantExpr(TheInt, *this))
+return std::make_pair(true, TheInt);
+ else
+return std::make_pair(false, TheInt);
+} else if (CAT) {
+return std::make_pair(true, CAT->getSize());
+} else {
+return std::make_pair(false, llvm::APInt());
+}
+ };
+
+ bool HaveLSize, HaveRSize;
+ llvm::APInt LSize, RSize;
+ std::tie(HaveLSize, LSize) = SizeFetch(LVAT, LCAT);
+ std::tie(HaveRSize, RSize) = SizeFetch(RVAT, RCAT);
+ if (HaveLSize && HaveRSize && LSize != RSize)
+return {}; // Definite, but unequal, array dimension
+}
+
if (LCAT && getCanonicalType(LHSElem) == getCanonicalType(ResultType))
return LHS;
if (RCAT && getCanonicalType(RHSElem) == getCanonicalType(ResultType))
return RHS;
if (LCAT) return getConstantArrayType(ResultType, LCAT->getSize(),
ArrayType::ArraySizeModifier(), 0);
if (RCAT) return getConstantArrayType(ResultType, RCAT->getSize(),
ArrayType::ArraySizeModifier(), 0);
-const VariableArrayType* LVAT = getAsVariableArrayType(LHS);
-const VariableArrayType* RVAT = getAsVariableArrayType(RHS);
if (LVAT && getCanonicalType(LHSElem) == getCanonicalType(ResultType))
return LHS;
if (RVAT && getCanonicalType(RHSElem) == getCanonicalType(ResultType))
Index: test/Sema/vla.c
===
--- test/Sema/vla.c
+++ test/Sema/vla.c
@@ -76,3 +76,16 @@
];
};
int (*use_implicitly_declared)() = implicitly_declared; // ok, was implicitly declared at file scope
+
+void VLAPtrAssign(int size) {
+ int array[1][2][3][size][4][5];
+ // This is well formed
+ int (*p)[2][3][size][4][5] = array;
+ // Last array dimension too large
+ int (*p2)[2][3][size][4][6] = array; // expected-warning {{incompatible pointer types}}
+ // Second array dimension too large
+ int (*p3)[20][3][size][4][5] = array; // expected-warning {{incompatible pointer types}}
+
+ // Not illegal in C, program _might_ be well formed if size == 3.
+ int (*p4)[2][size][3][4][5] = array;
+}
Index: lib/AST/ASTContext.cpp
==
[PATCH] D46667: [OpenCL, OpenMP] Fix crash when OpenMP used in OpenCL file
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333746: [OpenCL, OpenMP] Fix crash when OpenMP used in
OpenCL file (authored by erichkeane, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46667?vs=147538&id=149441#toc
Repository:
rC Clang
https://reviews.llvm.org/D46667
Files:
include/clang/AST/DeclOpenMP.h
lib/AST/DeclOpenMP.cpp
Index: lib/AST/DeclOpenMP.cpp
===
--- lib/AST/DeclOpenMP.cpp
+++ lib/AST/DeclOpenMP.cpp
@@ -92,13 +92,14 @@
OMPCapturedExprDecl *OMPCapturedExprDecl::Create(ASTContext &C, DeclContext
*DC,
IdentifierInfo *Id, QualType
T,
SourceLocation StartLoc) {
- return new (C, DC) OMPCapturedExprDecl(C, DC, Id, T, StartLoc);
+ return new (C, DC) OMPCapturedExprDecl(
+ C, DC, Id, T, C.getTrivialTypeSourceInfo(T), StartLoc);
}
OMPCapturedExprDecl *OMPCapturedExprDecl::CreateDeserialized(ASTContext &C,
unsigned ID) {
- return new (C, ID)
- OMPCapturedExprDecl(C, nullptr, nullptr, QualType(), SourceLocation());
+ return new (C, ID) OMPCapturedExprDecl(C, nullptr, nullptr, QualType(),
+ /*TInfo=*/nullptr, SourceLocation());
}
SourceRange OMPCapturedExprDecl::getSourceRange() const {
Index: include/clang/AST/DeclOpenMP.h
===
--- include/clang/AST/DeclOpenMP.h
+++ include/clang/AST/DeclOpenMP.h
@@ -189,8 +189,9 @@
void anchor() override;
OMPCapturedExprDecl(ASTContext &C, DeclContext *DC, IdentifierInfo *Id,
- QualType Type, SourceLocation StartLoc)
- : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, nullptr,
+ QualType Type, TypeSourceInfo *TInfo,
+ SourceLocation StartLoc)
+ : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, TInfo,
SC_None) {
setImplicit();
}
Index: lib/AST/DeclOpenMP.cpp
===
--- lib/AST/DeclOpenMP.cpp
+++ lib/AST/DeclOpenMP.cpp
@@ -92,13 +92,14 @@
OMPCapturedExprDecl *OMPCapturedExprDecl::Create(ASTContext &C, DeclContext *DC,
IdentifierInfo *Id, QualType T,
SourceLocation StartLoc) {
- return new (C, DC) OMPCapturedExprDecl(C, DC, Id, T, StartLoc);
+ return new (C, DC) OMPCapturedExprDecl(
+ C, DC, Id, T, C.getTrivialTypeSourceInfo(T), StartLoc);
}
OMPCapturedExprDecl *OMPCapturedExprDecl::CreateDeserialized(ASTContext &C,
unsigned ID) {
- return new (C, ID)
- OMPCapturedExprDecl(C, nullptr, nullptr, QualType(), SourceLocation());
+ return new (C, ID) OMPCapturedExprDecl(C, nullptr, nullptr, QualType(),
+ /*TInfo=*/nullptr, SourceLocation());
}
SourceRange OMPCapturedExprDecl::getSourceRange() const {
Index: include/clang/AST/DeclOpenMP.h
===
--- include/clang/AST/DeclOpenMP.h
+++ include/clang/AST/DeclOpenMP.h
@@ -189,8 +189,9 @@
void anchor() override;
OMPCapturedExprDecl(ASTContext &C, DeclContext *DC, IdentifierInfo *Id,
- QualType Type, SourceLocation StartLoc)
- : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, nullptr,
+ QualType Type, TypeSourceInfo *TInfo,
+ SourceLocation StartLoc)
+ : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, TInfo,
SC_None) {
setImplicit();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333746 - [OpenCL, OpenMP] Fix crash when OpenMP used in OpenCL file
Author: erichkeane
Date: Fri Jun 1 06:04:26 2018
New Revision: 333746
URL: http://llvm.org/viewvc/llvm-project?rev=333746&view=rev
Log:
[OpenCL, OpenMP] Fix crash when OpenMP used in OpenCL file
Compiler crashes when omp simd is used in an OpenCL file:
clang -c -fopenmp omp_simd.cl
__kernel void test(global int *data, int size) {
#pragma omp simd
for (int i = 0; i < size; ++i) {
}
}
The problem seems to be the check added to verify block pointers have
initializers. An OMPCapturedExprDecl is created to capture ‘size’ but there is
no TypeSourceInfo.
The change just uses getType() directly.
Patch-By: mikerice
Differential Revision: https://reviews.llvm.org/D46667
Modified:
cfe/trunk/include/clang/AST/DeclOpenMP.h
cfe/trunk/lib/AST/DeclOpenMP.cpp
Modified: cfe/trunk/include/clang/AST/DeclOpenMP.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclOpenMP.h?rev=333746&r1=333745&r2=333746&view=diff
==
--- cfe/trunk/include/clang/AST/DeclOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/DeclOpenMP.h Fri Jun 1 06:04:26 2018
@@ -189,8 +189,9 @@ class OMPCapturedExprDecl final : public
void anchor() override;
OMPCapturedExprDecl(ASTContext &C, DeclContext *DC, IdentifierInfo *Id,
- QualType Type, SourceLocation StartLoc)
- : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, nullptr,
+ QualType Type, TypeSourceInfo *TInfo,
+ SourceLocation StartLoc)
+ : VarDecl(OMPCapturedExpr, C, DC, StartLoc, StartLoc, Id, Type, TInfo,
SC_None) {
setImplicit();
}
Modified: cfe/trunk/lib/AST/DeclOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclOpenMP.cpp?rev=333746&r1=333745&r2=333746&view=diff
==
--- cfe/trunk/lib/AST/DeclOpenMP.cpp (original)
+++ cfe/trunk/lib/AST/DeclOpenMP.cpp Fri Jun 1 06:04:26 2018
@@ -92,13 +92,14 @@ void OMPCapturedExprDecl::anchor() {}
OMPCapturedExprDecl *OMPCapturedExprDecl::Create(ASTContext &C, DeclContext
*DC,
IdentifierInfo *Id, QualType
T,
SourceLocation StartLoc) {
- return new (C, DC) OMPCapturedExprDecl(C, DC, Id, T, StartLoc);
+ return new (C, DC) OMPCapturedExprDecl(
+ C, DC, Id, T, C.getTrivialTypeSourceInfo(T), StartLoc);
}
OMPCapturedExprDecl *OMPCapturedExprDecl::CreateDeserialized(ASTContext &C,
unsigned ID) {
- return new (C, ID)
- OMPCapturedExprDecl(C, nullptr, nullptr, QualType(), SourceLocation());
+ return new (C, ID) OMPCapturedExprDecl(C, nullptr, nullptr, QualType(),
+ /*TInfo=*/nullptr, SourceLocation());
}
SourceRange OMPCapturedExprDecl::getSourceRange() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric updated this revision to Diff 149446.
ioeric marked an inline comment as done.
ioeric edited the summary of this revision.
ioeric added a comment.
- Addressed review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,31 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,18 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// If OrigD is an declaration associated with a friend declaration and it's
+// not a definition, skip it. Note that OrigD is the occurrence that the
+// collector is currently visiting.
+if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+!(Roles & static_cast(index::SymbolRole::Definition)))
+ return true;
+D = ASTNode.OrigD;
+ }
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,31 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,18 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// If OrigD is an declaration associated with a friend declaration and it's
+// not a definition, skip it. Note that OrigD is the occurrence that the
+// collector is currently visiting.
+if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+!(Roles & static_cast(index::SymbolRole::Definition)))
+ return true;
+D = ASTNode.OrigD;
+ }
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.
ebevhan created this revision.
ebevhan added a reviewer: Anastasia.
Herald added a subscriber: cfe-commits.
The comment with the OpenCL clause about this clearly
says: "No type shall be qualified by qualifiers for
two or more different address spaces."
This must mean that two or more qualifiers for the
_same_ address space is allowed.
For dependent address space types, reject them like
before since we cannot know what the address space
will be.
Repository:
rC Clang
https://reviews.llvm.org/D47630
Files:
lib/Sema/SemaType.cpp
test/Sema/address_spaces.c
test/SemaOpenCL/address-spaces.cl
Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -62,4 +62,6 @@
__local __private int *var2; // expected-error {{multiple address spaces
specified for type}}
__local private_int_t var3; // expected-error {{multiple address spaces
specified for type}}
__local private_int_t *var4; // expected-error {{multiple address spaces
specified for type}}
+ __private private_int_t var5;
+ __private private_int_t *var6;
}
Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -14,6 +14,7 @@
int _AS1 _AS2 *Y; // expected-error {{multiple address spaces specified
for type}}
int *_AS1 _AS2 *Z; // expected-error {{multiple address spaces specified
for type}}
+ int *_AS1 _AS1 *M;
_AS1 int local; // expected-error {{automatic variable qualified with an
address space}}
_AS1 int array[5]; // expected-error {{automatic variable qualified with an
address space}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5703,14 +5703,6 @@
SourceLocation AttrLoc) {
if (!AddrSpace->isValueDependent()) {
-// If this type is already address space qualified, reject it.
-// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
-// by qualifiers for two or more different address spaces."
-if (T.getAddressSpace() != LangAS::Default) {
- Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
- return QualType();
-}
-
llvm::APSInt addrSpace(32);
if (!AddrSpace->isIntegerConstantExpr(addrSpace, Context)) {
Diag(AttrLoc, diag::err_attribute_argument_type)
@@ -5741,6 +5733,16 @@
LangAS ASIdx =
getLangASFromTargetAS(static_cast(addrSpace.getZExtValue()));
+// If this type is already address space qualified with a different
+// address space, reject it.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
+// by qualifiers for two or more different address spaces."
+if (T.getAddressSpace() != LangAS::Default &&
+T.getAddressSpace() != ASIdx) {
+ Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
+ return QualType();
+}
+
return Context.getAddrSpaceQualType(T, ASIdx);
}
@@ -5762,15 +5764,6 @@
/// space for the type.
static void HandleAddressSpaceTypeAttribute(QualType &Type,
const AttributeList &Attr, Sema
&S){
- // If this type is already address space qualified, reject it.
- // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified by
- // qualifiers for two or more different address spaces."
- if (Type.getAddressSpace() != LangAS::Default) {
-S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
-Attr.setInvalid();
-return;
- }
-
// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
// qualified by an address-space qualifier."
if (Type->isFunctionType()) {
@@ -5833,6 +5826,17 @@
llvm_unreachable("Invalid address space");
}
+// If this type is already address space qualified with a different
+// address space, reject it.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
by
+// qualifiers for two or more different address spaces."
+if (Type.getAddressSpace() != LangAS::Default &&
+Type.getAddressSpace() != ASIdx) {
+ S.Diag(Attr.getLoc(), diag::err_attribute_address_multiple_qualifiers);
+ Attr.setInvalid();
+ return;
+}
+
Type = S.Context.getAddrSpaceQualType(Type, ASIdx);
}
}
Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -62,4 +62,6 @@
__local __private int *var2; // expected-error {{multiple address spaces specified for type}}
__local private_int_t var3; // expected-error {{multiple address spaces specified for type}}
__local private_int_t *var4; // expected-erro
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
sammccall wrote:
> this seems suspect, we're going to treat the third decl in `friend X; X;
> friend X` differently from that in `X; friend X; friend X;`.
>
> Why? i.e. why is the inner check necessary and why does it treat the original
> (meaning first, I think) decl specially?
> this seems suspect, we're going to treat the third decl in `friend X; X;
> friend X` differently from that in `X; friend X; friend X;`.
I'm not very sure if I understand the problem. But I'll try to explain what
would happen for these two examples.
- For the first example, the first friend decl will be the canonical decl, and
we would only index the second `X` since its `OrigD` is not in friend decl.
Both the first and third friend decl will not be indexed.
- For the second example, the first non-friend `X` will be the canonical decl,
and all three occurrences will have the same `D` pointing to it. This probably
means that the same X will be processed three times, but it's probably fine (we
might want to optimize it).
Basically, `D` is always the canonical declaration in AST and `OrigD` is the
declaration that the indexer is currently visiting. I agree it's confusing...
> Why? i.e. why is the inner check necessary and why does it treat the original
> (meaning first, I think) decl specially?
The inner check handles the following case:
```
class X {
friend void foo();
}
void foo();
```
There will be two occurrences of `foo` in the index:
1. The friend decl, where `D` will be the canonical decl (i.e. friend foo) and
`OrigD` will also be friend foo.
2. The non-friend decl, where `D` will still be the canonical decl (i.e. friend
foo) and `OrigD` is now the non-friend decl.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:816
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
sammccall wrote:
> Can you also test that:
> - if a friend decl (non-definition) comes first, followed by a non-friend
> decl (non-definition), then the decl *is* indexed. (maybe just drop the
> definition from foo, since it's otherwise the same as Y)
> - if a friend decl has a definition, and there is no other declaration, then
> the decl *is* indexed (and the friend decl is canonical)
Done. I have missed the case where friend decl is a definition. Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington updated this revision to Diff 149449.
erik.pilkington marked 10 inline comments as done.
erik.pilkington added a comment.
Address review comments. Thanks!
https://reviews.llvm.org/D47607
Files:
libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/map
libcxx/include/unordered_map
Index: libcxx/include/unordered_map
===
--- libcxx/include/unordered_map
+++ libcxx/include/unordered_map
@@ -396,7 +396,7 @@
const _Hash& hash_function() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return static_cast(*this)(__x.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return static_cast(*this)(__x);}
@@ -425,7 +425,7 @@
const _Hash& hash_function() const _NOEXCEPT {return __hash_;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return __hash_(__x.__cc.first);}
+{return __hash_(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return __hash_(__x);}
@@ -464,13 +464,13 @@
const _Pred& key_eq() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return static_cast(*this)(__x.__cc.first, __y.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return static_cast(*this)(__x.__cc.first, __y);}
+{return static_cast(*this)(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return static_cast(*this)(__x, __y.__cc.first);}
+{return static_cast(*this)(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -496,13 +496,13 @@
const _Pred& key_eq() const _NOEXCEPT {return __pred_;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return __pred_(__x.__cc.first, __y.__cc.first);}
+{return __pred_(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return __pred_(__x.__cc.first, __y);}
+{return __pred_(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return __pred_(__x, __y.__cc.first);}
+{return __pred_(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -572,42 +572,88 @@
void operator()(pointer __p) _NOEXCEPT
{
if (__second_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.second));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().second));
if (__first_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.first));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().first));
if (__p)
__alloc_traits::deallocate(__na_, __p, 1);
}
};
#ifndef _LIBCPP_CXX03_LANG
template
-union __hash_value_type
+struct __hash_value_type
{
typedef _Key key_type;
typedef _Tp mapped_type;
typedef pairvalue_type;
-typedef pair __nc_value_type;
+typedef pair__nc_ref_pair_type;
+typedef pair __nc_rref_pair_type;
+private:
value_type __cc;
-__nc_value_type __nc;
+
+public:
+_LIBCPP_INLINE_VISIBILITY
+value_type& __get_value()
+{
+#if _LIBCPP_STD_VER > 14
+return *_VSTD::launder(_VSTD::addressof(__cc));
+#else
+return __cc;
+#endif
+}
+
+_LIBCPP_INLINE_VISIBILITY
+const value_type& __get_value() const
+{
+#if _LIBCPP_STD_VER > 14
+return *_VSTD::launder(_VSTD::addressof(__cc));
+#else
+return __cc;
+#endif
+}
+
+_LIBCPP_INLINE_VISIBILITY
+__nc_ref_pair_type __ref()
+{
+value_type& __v = __get_value();
+return __nc_ref_pair_type(const_cast(__v.first), __v.second);
+}
+
+_LIBCPP_INLINE_VISIBILITY
+__nc_rref_pair_type __move()
+{
+value_type& __v = __get_value();
+return __nc_rref_pair_type(
+_VSTD::move(const_cast(__v.first)),
+_VSTD::move(__v.second));
+}
_LIBCPP_INLINE_VISIBILITY
__hash_value_type& operator=(const __hash_value_type& __v)
-{__nc = __v.__cc; return *
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington added a comment.
In https://reviews.llvm.org/D47607#1118547, @EricWF wrote:
> I should have asked, have we actually seen a midcompile caused by this? Is
> there a reproducer? Or is this purerly speculative?
Nope, pure speculation. I still think we should still fix this though.
Comment at: libcxx/include/map:648
+_LIBCPP_INLINE_VISIBILITY
+__nc_ref_pair_type __ref()
+{
EricWF wrote:
> I think `__ref` can be private.
That's true for this patch, but I'm planning on using `__ref` in
`__node_handle` to implement key() and mapped() so we don't have to duplicate
the const cast.
https://reviews.llvm.org/D47607
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47632: [ASTImporter] Refactor Decl creation
martong created this revision.
martong added reviewers: a.sidorin, balazske, xazax.hun, r.stahl.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.
Generalize the creation of Decl nodes during Import. With this patch we do the
same things after and before a new AST node is created (::Create) The import
logic should be really simple, we create the node, then we mark that as
imported, then we recursively import the parts for that node and then set them
on that node. However, the AST is actually a graph, so we have to handle
circles. If we mark something as imported (`MapImported()`) then we return with
the corresponding `To` decl whenever we want to import that node again, this way
circles are handled. In order to make this algorithm work we must ensure
things, which are handled in the generic CreateDecl<> template:
- There are no `Import()` calls in between any node creation (::Create)
and the `MapImported()` call.
- Before actually creating an AST node (::Create), we must check if
the Node had been imported already, if yes then return with that one.
One very important case for this is connected to templates: we may
start an import both from the templated decl of a template and from
the template itself.
Now, the virtual `Imported` function is called in `ASTImporter::Impor(Decl *)`,
but only once, when the `Decl` is imported. One point of this refactor is to
separate responsibilities. The original `Imported()` had 3 responsibilities:
- notify subclasses when an import happened
- register the decl into `ImportedDecls`
- initialise the Decl (set attributes, etc)
Now all of these are in separate functions:
- `Imported`
- `MapImported`
- `InitializeImportedDecl`
I tried to check all the clients, I executed tests for `ExternalASTMerger.cpp`
and some unittests for lldb.
Repository:
rC Clang
https://reviews.llvm.org/D47632
Files:
include/clang/AST/ASTImporter.h
include/clang/AST/DeclBase.h
lib/AST/ASTImporter.cpp
lib/AST/ExternalASTMerger.cpp
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -154,7 +154,7 @@
ToContainer->setMustBuildLookupTable();
assert(Parent.CanComplete(ToContainer));
}
-return ASTImporter::Imported(From, To);
+return To;
}
ASTImporter &GetReverse() { return Reverse; }
};
@@ -229,7 +229,7 @@
SourceTag->getASTContext().getExternalSource()->CompleteType(SourceTag);
if (!SourceTag->getDefinition())
return false;
-Forward.Imported(SourceTag, Tag);
+Forward.MapImported(SourceTag, Tag);
Forward.ImportDefinition(SourceTag);
Tag->setCompleteDefinition(SourceTag->isCompleteDefinition());
return true;
@@ -248,7 +248,7 @@
SourceInterface);
if (!SourceInterface->getDefinition())
return false;
-Forward.Imported(SourceInterface, Interface);
+Forward.MapImported(SourceInterface, Interface);
Forward.ImportDefinition(SourceInterface);
return true;
});
@@ -304,7 +304,7 @@
void ExternalASTMerger::RecordOriginImpl(const DeclContext *ToDC, DCOrigin Origin,
ASTImporter &Importer) {
Origins[ToDC] = Origin;
- Importer.ASTImporter::Imported(cast(Origin.DC), const_cast(cast(ToDC)));
+ Importer.ASTImporter::MapImported(cast(Origin.DC), const_cast(cast(ToDC)));
}
ExternalASTMerger::ExternalASTMerger(const ImporterTarget &Target,
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -76,6 +76,58 @@
public StmtVisitor {
ASTImporter &Importer;
+// Wrapper for an overload set.
+template struct CallOverloadedCreateFun {
+ template
+ auto operator()(Args &&... args)
+ -> decltype(ToDeclT::Create(std::forward(args)...)) {
+return ToDeclT::Create(std::forward(args)...);
+ }
+};
+
+// Always use this function to create a Decl during import. There are
+// certain tasks which must be done after the Decl was created, e.g. we
+// must immediately register that as an imported Decl.
+// Returns a pair consisting of a pointer to the new or the already imported
+// Decl and a bool value set to true if the `FromD` had been imported
+// before.
+template
+std::pair CreateDecl(FromDeclT *FromD, Args &&... args) {
+ // There may be several overloads of ToDeclT::Create. We must make sure
+ // to call the one which would be chosen by the arguments, thus we use a
+ // wrapper for the overload set.
+ CallOverloadedCreateFun OC;
+ return CreateDecl(OC, FromD, std::forward(args)...);
+}
+// Use this overload directly only if a special create function must be
+// used, e.g. CXXRecordDecl::CreateLambda .
+template
+auto CreateDecl(Cre
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ilya-biryukov added a comment.
In https://reviews.llvm.org/D47623#1118810, @sammccall wrote:
> - friend decls that are not definitions should be ignored for indexing
> purposes
This is not generally true IIUC. A friend declaration can be a reference, a
declaration or a definition.
int foo(int);
int bar(int, int);
class ExistingFwdCls;
class X {
friend class ExistingFwdCls; // a reference and a declaration.
friend class NewClass; // a new declaration.
friend int foo(int); // a reference and a declaration.
friend int baz(int, int, int); // a new declaration.
};
class Y {
friend class ::ExistingFwdCls; // qualified => just a reference.
friend int ::bar(int a, int b); // qualified => just a reference.
friend int foo(int) { // a reference and a definition
return 100;
}
};
Note that friend functions with bodies are probably ok as canonical
declaration, as they are often the only declaration, e.g.
class Foo {
friend bool operator < (Foo const& lhs, Foo const&lhs) {
return false;
}
};
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
ioeric wrote:
> ilya-biryukov wrote:
> > Maybe move this closer to `shouldFilterDecl()`? We have similar filters
> > there.
> > That would also mean we properly add the reference counts for friend
> > declarations that get a normal declaration after their usage later.
> I didn't put this filter there because I think it's a bit more special than
> those filters in `shouldFilterDecl`. We check the `OrigD` and we could
> potentially replace `D` with `OrigD`. We could change `shouldFilterDecl` to
> handle that, but I'm not sure if it's worth it.
>
> Reference counting for friend declarations is actually a bit tricky as USRs
> of the generated declarations might be ambiguous.
>
> Consider the following exmaple:
> ```
> namespace a {
> class A {};
> namespace b { class B { friend class A; }; } // b
> } // a
> ```
>
> I would expect the generated friend decl to be `a::A`, but it's actually
> `a::b::A`! So getting USR right is a bit tricky, and I think it's probably ok
> to ignore references in friend decls.
>
> For reference, `[namespace.memdef]p3`:
> "If the name in a friend declaration is neither qualified nor a template-id
> and the declaration is a function or an elaborated-type-specifier, the lookup
> to determine whether the entity has been previously declared shall not
> consider any scopes outside the innermost enclosing namespace.
> Reference counting for friend declarations is actually a bit tricky as USRs
> of the generated declarations might be ambiguous.
This seems like an obvious bug in the USRs that we should fix. WDYT?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
sammccall added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
ioeric wrote:
> sammccall wrote:
> > this seems suspect, we're going to treat the third decl in `friend X; X;
> > friend X` differently from that in `X; friend X; friend X;`.
> >
> > Why? i.e. why is the inner check necessary and why does it treat the
> > original (meaning first, I think) decl specially?
> > this seems suspect, we're going to treat the third decl in `friend X; X;
> > friend X` differently from that in `X; friend X; friend X;`.
> I'm not very sure if I understand the problem. But I'll try to explain what
> would happen for these two examples.
> - For the first example, the first friend decl will be the canonical decl,
> and we would only index the second `X` since its `OrigD` is not in friend
> decl. Both the first and third friend decl will not be indexed.
> - For the second example, the first non-friend `X` will be the canonical
> decl, and all three occurrences will have the same `D` pointing to it. This
> probably means that the same X will be processed three times, but it's
> probably fine (we might want to optimize it).
>
> Basically, `D` is always the canonical declaration in AST and `OrigD` is the
> declaration that the indexer is currently visiting. I agree it's confusing...
>
> > Why? i.e. why is the inner check necessary and why does it treat the
> > original (meaning first, I think) decl specially?
>
> The inner check handles the following case:
> ```
> class X {
> friend void foo();
> }
> void foo();
> ```
>
> There will be two occurrences of `foo` in the index:
> 1. The friend decl, where `D` will be the canonical decl (i.e. friend foo)
> and `OrigD` will also be friend foo.
> 2. The non-friend decl, where `D` will still be the canonical decl (i.e.
> friend foo) and `OrigD` is now the non-friend decl.
> Basically, D is always the canonical declaration in AST and OrigD is the
> declaration that the indexer is currently visiting. I agree it's confusing...
Whoops, I had this backwards. So I guess I mean the outer check.
Fundamentally my question is: in an *arbitrary* list of redecls of a symbol,
what is special about the canonical declaration (D, which is just the first in
the list) that we particularly care whether it's a friend?
I would expect that either we're ignoring the other redecls, or we're looping
over all redecls.
And here, I think we could just skip all friend decls (that are not
definitions), regardless of what `D->getFriendObjectKind()` is. If we have
other decls, the symbol was indexed already. If we do not, then we don't want
to index it anyway.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > Maybe move this closer to `shouldFilterDecl()`? We have similar filters
> > > there.
> > > That would also mean we properly add the reference counts for friend
> > > declarations that get a normal declaration after their usage later.
> > I didn't put this filter there because I think it's a bit more special than
> > those filters in `shouldFilterDecl`. We check the `OrigD` and we could
> > potentially replace `D` with `OrigD`. We could change `shouldFilterDecl` to
> > handle that, but I'm not sure if it's worth it.
> >
> > Reference counting for friend declarations is actually a bit tricky as USRs
> > of the generated declarations might be ambiguous.
> >
> > Consider the following exmaple:
> > ```
> > namespace a {
> > class A {};
> > namespace b { class B { friend class A; }; } // b
> >
> > } // a
> > ```
> >
> > I would expect the generated friend decl to be `a::A`, but it's actually
> > `a::b::A`! So getting USR right is a bit tricky, and I think it's probably
> > ok to ignore references in friend decls.
> >
> > For reference, `[namespace.memdef]p3`:
> > "If the name in a friend declaration is neither qualified nor a template-id
> > and the declaration is a function or an elaborated-type-specifier, the
> > lookup to determine whether the entity has been previously declared shall
> > not consider any scopes outside the innermost enclosing namespace.
> > Reference counting for friend declarations is actually a bit tricky as USRs
> > of the generated declarations might be ambiguous.
> This seems like an obvious bug in the USRs that we should fix. WDYT?
Sorry, I think I was not clear... I think this is intended according to the
standard. So in the example, the qualified name of the friend decl `a::b::A`
is, although confusing, correct, and o the actual problem is not with the USR.
See `[namespace.memdef]p3`:
"If the name in a friend declaration is neither qualified nor a template-id and
the declaration is a function or an elaborated-type-specifier, the lookup to
determine whether the entity has been previously declared shall not consider
any scopes outside the innermost enclosing namespace.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
gtbercea added a comment. > I disagree in this context because this patch currently means that static > archives will only work with NVPTX and there is no clear path how to "fix" > things for other offloading targets. I'll try to work on my proposal over the > next few days (sorry, very busy week...), maybe I can put together a > prototype of my idea. Other toolchains can also have static linking if they: 1. ditch the clang-offload-bundler for generating/consuming object files. 2. implement a link step on the device toolchain which can identify the vendor specific object file inside the host object file. (this is how the so called "bunlding" should have been done in the first place not using a custom tool which limits the functionality of the compiler). Identifying toolchain-specific objects/binaries is a task that belongs within the device-specific toolchain and SHOULD NOT be factored out because you can't treat object that are different by definition in the same way. Ignoring their differences leads to those object not being link-able. On top of that, factoring out introduces custom object formats which only CLANG understands AND it introduces compilation steps that impede static linking. I'm surprised you now disagree with this technique, when I first introduced you to this in an e-mail off list you agreed with it. So this patch, the only new CUDA tool that it calls is the FATBINARY tool which is done on the device-specific side of the toolchain so you can't possibly object to that. The CUDA toolchain already calls FATBINARY so it's not a novelty. That step is essential to making device-side objects identifiable by NVLINK (which we already call). The only step you might object to is the partial linking step which, as I explained in my original post, I envisage will be improved over time as more toolchains support this scheme. I think this is a true solution to the problem. What you are proposing is a workaround that doesn't tackle the problem head-on. Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
takuto.ikuta accepted this revision. takuto.ikuta added a comment. I confirmed this CL and https://reviews.llvm.org/D47578 remove absolute path from /showIncludes when include paths are given in relative. https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added inline comments. Comment at: libcxx/include/__hash_table:2261 +_NodeHandle +__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_extract_unique( +key_type const& __key) EricWF wrote: > If I'm not mistaken, `__node_handle_extract_unique` and > `__node_handle_extract_multi` have the exact same implementation. This is > intentional, no? If so can't we just use one implementation? Yep, good point. The new patch just has `__node_handle_extract`. https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
Hahnfeld added a comment. In https://reviews.llvm.org/D47394#1118957, @gtbercea wrote: > I'm surprised you now disagree with this technique, when I first introduced > you to this in an e-mail off list you agreed with it. My words were `I agree this is the best solution for NVPTX.` In the same reply I asked how your proposal is supposed to work for other offloading targets which is now clear to require additional work, maybe even completely novel tools. So now I disagree that it is the **right** solution for Clang because I think my proposal will cover all offloading targets. Please give me a bit time so that I can see if it works. Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:282-290
+def warn_unused_using_declaration : Warning<
+ "unused using declaration %0">,
+ InGroup, DefaultIgnore;
+def warn_unused_using_directive : Warning<
+ "unused using directive %0">,
+ InGroup, DefaultIgnore;
+def warn_unused_using_alias : Warning<
lebedev.ri wrote:
> JFYI you can condense it down to just
> ```
> def warn_unused_using_declaration : Warning<
> "unused %select{using declaration|using directive|namespace alias}0 %1">,
> InGroup, DefaultIgnore;
> ```
> if that simplifies the code that actually emits that warning.
Thanks for the suggestion.
I will have a look at it and see if that simplifies the code that emits the
warning.
https://reviews.llvm.org/D44826
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333752 - Fix unused variable warning from r333718
Author: erichkeane
Date: Fri Jun 1 07:16:18 2018
New Revision: 333752
URL: http://llvm.org/viewvc/llvm-project?rev=333752&view=rev
Log:
Fix unused variable warning from r333718
Modified:
cfe/trunk/lib/Lex/ModuleMap.cpp
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=333752&r1=333751&r2=333752&view=diff
==
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Jun 1 07:16:18 2018
@@ -234,7 +234,7 @@ const FileEntry *ModuleMap::findHeader(
// framework style path.
FullPathName.assign(Directory->getName());
RelativePathName.clear();
-if (auto *FrameworkHdr = GetFrameworkFile()) {
+if (GetFrameworkFile()) {
Diags.Report(Header.FileNameLoc,
diag::warn_mmap_incomplete_framework_module_declaration)
<< Header.FileName << M->getFullModuleName();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.
bader added inline comments.
Comment at: test/Sema/address_spaces.c:17
int *_AS1 _AS2 *Z; // expected-error {{multiple address spaces specified
for type}}
+ int *_AS1 _AS1 *M;
I think it might be valuable to give a warning or remark to user.
Using the same address space qualifier multiple times is not something OpenCL C
developers are supposed to do.
Repository:
rC Clang
https://reviews.llvm.org/D47630
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric updated this revision to Diff 149465.
ioeric added a comment.
- Clarify.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,31 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,18 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ assert(ASTNode.OrigD);
+ // If OrigD is an declaration associated with a friend declaration and it's
+ // not a definition, skip it. Note that OrigD is the occurrence that the
+ // collector is currently visiting.
+ if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+ !(Roles & static_cast(index::SymbolRole::Definition)))
+return true;
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
+D = ASTNode.OrigD;
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,31 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,18 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ assert(ASTNode.OrigD);
+ // If OrigD is an declaration associated with a friend declaration and it's
+ // not a definition, skip it. Note that OrigD is the occurrence that the
+ // collector is currently visiting.
+ if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+ !(Roles & static_cast(index::SymbolRole::Definition)))
+return true;
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
+D = ASTNode.OrigD;
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > this seems suspect, we're going to treat the third decl in `friend X; X;
> > > friend X` differently from that in `X; friend X; friend X;`.
> > >
> > > Why? i.e. why is the inner check necessary and why does it treat the
> > > original (meaning first, I think) decl specially?
> > > this seems suspect, we're going to treat the third decl in `friend X; X;
> > > friend X` differently from that in `X; friend X; friend X;`.
> > I'm not very sure if I understand the problem. But I'll try to explain what
> > would happen for these two examples.
> > - For the first example, the first friend decl will be the canonical decl,
> > and we would only index the second `X` since its `OrigD` is not in friend
> > decl. Both the first and third friend decl will not be indexed.
> > - For the second example, the first non-friend `X` will be the canonical
> > decl, and all three occurrences will have the same `D` pointing to it. This
> > probably means that the same X will be processed three times, but it's
> > probably fine (we might want to optimize it).
> >
> > Basically, `D` is always the canonical declaration in AST and `OrigD` is
> > the declaration that the indexer is currently visiting. I agree it's
> > confusing...
> >
> > > Why? i.e. why is the inner check necessary and why does it treat the
> > > original (meaning first, I think) decl specially?
> >
> > The inner check handles the following case:
> > ```
> > class X {
> > friend void foo();
> > }
> > void foo();
> > ```
> >
> > There will be two occurrences of `foo` in the index:
> > 1. The friend decl, where `D` will be the canonical decl (i.e. friend foo)
> > and `OrigD` will also be friend foo.
> > 2. The non-friend decl, where `D` will still be the canonical decl (i.e.
> > friend foo) and `OrigD` is now the non-friend decl.
> > Basically, D is always the canonical declaration in AST and OrigD is the
> > declaration that the indexer is currently visiting. I agree it's
> > confusing...
>
> Whoops, I had this backwards. So I guess I mean the outer check.
> Fundamentally my question is: in an *arbitrary* list of redecls of a symbol,
> what is special about the canonical declaration (D, which is just the first
> in the list) that we particularly care whether it's a friend?
> I would expect that either we're ignoring the other redecls, or we're looping
> over all redecls.
>
> And here, I think we could just skip all friend decls (that are not
> definitions), regardless of what `D->getFriendObjectKind()` is. If we have
> other decls, the symbol was indexed already. If we do not, then we don't want
> to index it anyway.
> Fundamentally my question is: in an *arbitrary* list of redecls of a symbol,
> what is special about the canonical declaration (D, which is just the first
> in the list) that we particularly care whether it's a friend?
It's because we have chosen to use `D` as a symbol's canonical declaration
(line 332). Here we want to override the canonical declaration `D` when it's a
friend decl, so that the first non-friend decl would become the canonical
declaration of the symbol.
> And here, I think we could just skip all friend decls (that are not
> definitions), regardless of what D->getFriendObjectKind() is.
OK. I think I understand where the confusion came from now. The override of `D`
should've been a separate check:
```
if (OrigD is non-definition friend)
skip;
if (D is friend decl)
D = OrigD;
```
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333757 - [OpenMP] Fix typo in NVPTX linker, NFC.
Author: hahnfeld
Date: Fri Jun 1 07:43:48 2018
New Revision: 333757
URL: http://llvm.org/viewvc/llvm-project?rev=333757&view=rev
Log:
[OpenMP] Fix typo in NVPTX linker, NFC.
Clang calls "nvlink" for linking multiple object files with OpenMP
target functions, so correct this information when printing errors.
Modified:
cfe/trunk/lib/Driver/ToolChains/Cuda.h
Modified: cfe/trunk/lib/Driver/ToolChains/Cuda.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Cuda.h?rev=333757&r1=333756&r2=333757&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Cuda.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Cuda.h Fri Jun 1 07:43:48 2018
@@ -115,7 +115,7 @@ class LLVM_LIBRARY_VISIBILITY Linker : p
class LLVM_LIBRARY_VISIBILITY OpenMPLinker : public Tool {
public:
OpenMPLinker(const ToolChain &TC)
- : Tool("NVPTX::OpenMPLinker", "fatbinary", TC, RF_Full,
llvm::sys::WEM_UTF8,
+ : Tool("NVPTX::OpenMPLinker", "nvlink", TC, RF_Full,
llvm::sys::WEM_UTF8,
"--options-file") {}
bool hasIntegratedCPP() const override { return false; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333758 - [clangd] Compute better estimates for memory usage of the AST
Author: ibiryukov
Date: Fri Jun 1 07:44:57 2018
New Revision: 333758
URL: http://llvm.org/viewvc/llvm-project?rev=333758&view=rev
Log:
[clangd] Compute better estimates for memory usage of the AST
Also fix the return value of IdleASTs::getUsedBytes().
It was 'bool' instead of 'size_t' *facepalm*.
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/TUScheduler.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333758&r1=333757&r2=333758&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Jun 1 07:44:57 2018
@@ -220,8 +220,32 @@ std::size_t ParsedAST::getUsedBytes() co
auto &AST = getASTContext();
// FIXME(ibiryukov): we do not account for the dynamically allocated part of
// Message and Fixes inside each diagnostic.
- return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
- ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags);
+ std::size_t Total =
+ ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags);
+
+ // FIXME: the rest of the function is almost a direct copy-paste from
+ // libclang's clang_getCXTUResourceUsage. We could share the implementation.
+
+ // Sum up variaous allocators inside the ast context and the preprocessor.
+ Total += AST.getASTAllocatedMemory();
+ Total += AST.getSideTableAllocatedMemory();
+ Total += AST.Idents.getAllocator().getTotalMemory();
+ Total += AST.Selectors.getTotalMemory();
+
+ Total += AST.getSourceManager().getContentCacheSize();
+ Total += AST.getSourceManager().getDataStructureSizes();
+ Total += AST.getSourceManager().getMemoryBufferSizes().malloc_bytes;
+
+ if (ExternalASTSource *Ext = AST.getExternalSource())
+Total += Ext->getMemoryBufferSizes().malloc_bytes;
+
+ const Preprocessor &PP = getPreprocessor();
+ Total += PP.getTotalMemory();
+ if (PreprocessingRecord *PRec = PP.getPreprocessingRecord())
+Total += PRec->getTotalMemory();
+ Total += PP.getHeaderSearchInfo().getTotalMemory();
+
+ return Total;
}
const std::vector &ParsedAST::getInclusions() const {
Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=333758&r1=333757&r2=333758&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Fri Jun 1 07:44:57 2018
@@ -75,7 +75,7 @@ public:
/// Returns result of getUsedBytes() for the AST cached by \p K.
/// If no AST is cached, 0 is returned.
- bool getUsedBytes(Key K) {
+ std::size_t getUsedBytes(Key K) {
std::lock_guard Lock(Mut);
auto It = findByKey(K);
if (It == LRU.end() || !It->second)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333761 - clang-cl: Expose -no-canonical-prefixes
Author: nico Date: Fri Jun 1 07:59:57 2018 New Revision: 333761 URL: http://llvm.org/viewvc/llvm-project?rev=333761&view=rev Log: clang-cl: Expose -no-canonical-prefixes -no-canonical-prefixes is a weird flag: In gcc, it controls whether realpath() is called on the path of the driver binary. It's needed to support some usecases where gcc is symlinked to, see https://gcc.gnu.org/ml/gcc/2011-01/msg00429.html for some background. In clang, the resource dir is found relative to the compiler binary, and without -no-canonical-prefixes that's an absolute path. For clang, the main use case for -no-canonical-prefixes is to make the -resource-dir path added by the driver relative instead of absolute. Making it relative seems like the better default, but since neither clang not gcc have -canonical-prefixes without no- which makes changing the default tricky, and since some symlink behaviors do depend on the realpath() call at least for gcc, just expose -no-canonical-prefixes in clang-cl mode. Alternatively we could default to no-canonical-prefix-mode for clang-cl since it's less likely to be used in symlinked scenarios, but since you already need to about -no-canonical-prefixes for the non-clang-cl bits of your build, not hooking this of driver mode seems better to me. https://reviews.llvm.org/D47480 Modified: cfe/trunk/include/clang/Driver/Options.td cfe/trunk/test/Driver/cl-options.c Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=333761&r1=333760&r2=333761&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Fri Jun 1 07:59:57 2018 @@ -2261,7 +2261,7 @@ def multi__module : Flag<["-"], "multi_m def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">; def multiply__defined : Separate<["-"], "multiply_defined">; def mwarn_nonportable_cfstrings : Flag<["-"], "mwarn-nonportable-cfstrings">, Group; -def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, Flags<[HelpHidden]>, +def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, Flags<[HelpHidden, CoreOption]>, HelpText<"Use relative instead of canonical paths">; def no_cpp_precomp : Flag<["-"], "no-cpp-precomp">, Group; def no_integrated_cpp : Flag<["-", "--"], "no-integrated-cpp">, Flags<[DriverOption]>; Modified: cfe/trunk/test/Driver/cl-options.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=333761&r1=333760&r2=333761&view=diff == --- cfe/trunk/test/Driver/cl-options.c (original) +++ cfe/trunk/test/Driver/cl-options.c Fri Jun 1 07:59:57 2018 @@ -591,6 +591,8 @@ // RUN: -flimit-debug-info \ // RUN: -flto \ // RUN: -fmerge-all-constants \ +// RUN: -no-canonical-prefixes \ +// RUN: -fno-coverage-mapping \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
Hahnfeld added a comment. Hmm, maybe the scope is much larger: I just tried linking an executable that references a `declare target` function in a shared library. My assumption was that this already works, given that `libomptarget`'s registration functions can be called multiple times. Am I doing something wrong? Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
thakis closed this revision. thakis added a comment. r333761, thanks! https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333762 - Remove redundant -fno-coverage-mapping added in r333761 (already added in r333423)
Author: nico Date: Fri Jun 1 08:02:43 2018 New Revision: 333762 URL: http://llvm.org/viewvc/llvm-project?rev=333762&view=rev Log: Remove redundant -fno-coverage-mapping added in r333761 (already added in r333423) Modified: cfe/trunk/test/Driver/cl-options.c Modified: cfe/trunk/test/Driver/cl-options.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=333762&r1=333761&r2=333762&view=diff == --- cfe/trunk/test/Driver/cl-options.c (original) +++ cfe/trunk/test/Driver/cl-options.c Fri Jun 1 08:02:43 2018 @@ -592,7 +592,6 @@ // RUN: -flto \ // RUN: -fmerge-all-constants \ // RUN: -no-canonical-prefixes \ -// RUN: -fno-coverage-mapping \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
thakis marked 2 inline comments as done. thakis added inline comments. Comment at: test/Driver/cl-options.c:595 +// RUN: -no-canonical-prefixes \ +// RUN: -fno-coverage-mapping \ // RUN: --version \ rnk wrote: > takuto.ikuta wrote: > > Is this related to this change? > It's a test for a previous change that I made. Sorry, I missed this. It was a mistake, I removed this line again in r333762. https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.
ebevhan added inline comments.
Comment at: test/Sema/address_spaces.c:17
int *_AS1 _AS2 *Z; // expected-error {{multiple address spaces specified
for type}}
+ int *_AS1 _AS1 *M;
bader wrote:
> I think it might be valuable to give a warning or remark to user.
> Using the same address space qualifier multiple times is not something OpenCL
> C developers are supposed to do.
>
The test is obviously a bit contrived, but it could happen by mistake, or as a
result of some typedef or macro combination. It also cannot go wrong, so
there's no harm in it happening.
I see your point, though. A warning feels like a bit much, so I'm not sure what
else to use. A note?
Repository:
rC Clang
https://reviews.llvm.org/D47630
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer
xazax.hun added a comment. Sorry for the limited activity. Unfortunately, I have very little time reviewing patches lately. I think we need to answer the following questions: - Does this change affect the analysis of the constructors of global objects? If so, how? - Do we want to import non-const object's initializer expressions? The analyzer might not end up using the value anyways. - How big can the index get with this modification for large projects? Overall the direction looks good to me, this will be a very useful addition, thanks for working on this. https://reviews.llvm.org/D46421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:223-225 + ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState()); + if (!Node) +return; Szelethus wrote: > NoQ wrote: > > Szelethus wrote: > > > NoQ wrote: > > > > I suspect that a fatal error is better here. We don't want the user to > > > > receive duplicate report from other checkers that catch uninitialized > > > > values; just one report is enough. > > > I think that would be a bad idea. For example, this checker shouts so > > > loudly while checking the LLVM project, it would practically halt the > > > analysis of the code, reducing the coverage, which means that checkers > > > other then uninit value checkers would "suffer" from it. > > > > > > However, I also think that having multiple uninit reports for the same > > > object might be good, especially with this checker, as it would be very > > > easy to see where the problem originated from. > > > > > > What do you think? > > Well, i guess that's the reason to not use the checker on LLVM. Regardless > > of fatal/nonfatal warnings, enabling this checker on LLVM regularly would > > be a bad idea because it's unlikely that anybody will be able to fix all > > the false positives to make it usable. And for other projects that don't > > demonstrate many false positives, this shouldn't be a problem. > > > > In order to indicate where the problem originates from, we have our bug > > reporter visitors that try their best to add such info directly to the > > report. In particular, @george.karpenkov's recent `NoStoreFuncVisitor` > > highlights functions in which a variable was //not// initialized but was > > probably expected to be. Not sure if it highlights constructors in its > > current shape, but that's definitely a better way to give this piece of > > information to the user because it doesn't make the user look for a > > different report to understand the current report. > LLVM is a special project in the fact that almost every part of it is so > performance critical, that leaving many fields uninit matters. However, I > would believe that in most projects, only a smaller portion of the code would > be like that. > > Suppose that we have a project that also defines a set of ADTs, like an > `std::list`-like container. If that container had a field that would be left > uninit after a ctor call, analysis on every execution path would be halted > which would use an object like that. > > My point is, as long as there is no way to tell the analyzer (or the checker) > to ignore certain constructor calls, I think it would be best not to generate > a fatal error. > > >Regardless of fatal/nonfatal warnings, enabling this checker on LLVM > >regularly would be a bad idea because it's unlikely that anybody will be > >able to fix all the false positives to make it usable. And for other > >projects that don't demonstrate many false positives, this shouldn't be a > >problem. > I wouldn't necessarily call them false positives. This checker doesn't look > for bugs, and all reports I checked were correct in the fact that those > fields really were left uninit. They just don't cause any trouble (just yet!). I think of this check as a tool to support a specific programming model where every field needs to be initialized by the constructor. This programming model might be followed by some parts of the projects while a 3rd party library in the same project or some other files might not follow this model. Right now there is no easy way to turn off some checks for a set of files, and there is no way to turn off a set of checks for some headers. For this reason, I think it is not a good idea to make these errors fatal, as 3rd party headers might reduce the coverage of the analysis on a project in a way that the user cannot control. If we are afraid of having multiple reports from this check we could turn off the check for that particular path, for example, we could have a bool stored in the GDM for each path whether this check is already reported an error or not and we can check that before emitting warnings. https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47628: Detect an incompatible VLA pointer assignment
efriedma added inline comments.
Comment at: lib/AST/ASTContext.cpp:8588
+ Expr *E = VAT->getSizeExpr();
+ if (E && VAT->getSizeExpr()->isIntegerConstantExpr(TheInt, *this))
+return std::make_pair(true, TheInt);
`E && E->isIntegerConstantExpr`?
Comment at: lib/AST/ASTContext.cpp:8603
+ std::tie(HaveRSize, RSize) = SizeFetch(RVAT, RCAT);
+ if (HaveLSize && HaveRSize && LSize != RSize)
+return {}; // Definite, but unequal, array dimension
The != will hit an assertion failure if LSize and RSize don't have the same
bitwidth.
Repository:
rC Clang
https://reviews.llvm.org/D47628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45202: [X86] Replacing X86-specific floor and ceil vector intrinsics with generic LLVM intrinsics
mike.dvoretsky updated this revision to Diff 149484.
mike.dvoretsky added a comment.
Changed the scalar intrinsic lowering to work via extract-insert.
https://reviews.llvm.org/D45203 contains tests for folding the resulting IR
patterns.
https://reviews.llvm.org/D45202
Files:
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/avx-builtins.c
clang/test/CodeGen/avx512f-builtins.c
clang/test/CodeGen/sse41-builtins.c
Index: clang/test/CodeGen/sse41-builtins.c
===
--- clang/test/CodeGen/sse41-builtins.c
+++ clang/test/CodeGen/sse41-builtins.c
@@ -44,25 +44,31 @@
__m128d test_mm_ceil_pd(__m128d x) {
// CHECK-LABEL: test_mm_ceil_pd
- // CHECK: call <2 x double> @llvm.x86.sse41.round.pd(<2 x double> %{{.*}}, i32 2)
+ // CHECK: @llvm.ceil.v2f64
+ // CHECK-NOT: select
return _mm_ceil_pd(x);
}
__m128 test_mm_ceil_ps(__m128 x) {
// CHECK-LABEL: test_mm_ceil_ps
- // CHECK: call <4 x float> @llvm.x86.sse41.round.ps(<4 x float> %{{.*}}, i32 2)
+ // CHECK: @llvm.ceil.v4f32
+ // CHECK-NOT: select
return _mm_ceil_ps(x);
}
__m128d test_mm_ceil_sd(__m128d x, __m128d y) {
// CHECK-LABEL: test_mm_ceil_sd
- // CHECK: call <2 x double> @llvm.x86.sse41.round.sd(<2 x double> %{{.*}}, <2 x double> %{{.*}}, i32 2)
+ // CHECK: extractelement
+ // CHECK: @llvm.ceil.f64
+ // CHECK: insertelement
return _mm_ceil_sd(x, y);
}
__m128 test_mm_ceil_ss(__m128 x, __m128 y) {
// CHECK-LABEL: test_mm_ceil_ss
- // CHECK: call <4 x float> @llvm.x86.sse41.round.ss(<4 x float> %{{.*}}, <4 x float> %{{.*}}, i32 2)
+ // CHECK: extractelement
+ // CHECK: @llvm.ceil.f32
+ // CHECK: insertelement
return _mm_ceil_ss(x, y);
}
@@ -196,25 +202,31 @@
__m128d test_mm_floor_pd(__m128d x) {
// CHECK-LABEL: test_mm_floor_pd
- // CHECK: call <2 x double> @llvm.x86.sse41.round.pd(<2 x double> %{{.*}}, i32 1)
+ // CHECK: @llvm.floor.v2f64
+ // CHECK-NOT: select
return _mm_floor_pd(x);
}
__m128 test_mm_floor_ps(__m128 x) {
// CHECK-LABEL: test_mm_floor_ps
- // CHECK: call <4 x float> @llvm.x86.sse41.round.ps(<4 x float> %{{.*}}, i32 1)
+ // CHECK: @llvm.floor.v4f32
+ // CHECK-NOT: select
return _mm_floor_ps(x);
}
__m128d test_mm_floor_sd(__m128d x, __m128d y) {
// CHECK-LABEL: test_mm_floor_sd
- // CHECK: call <2 x double> @llvm.x86.sse41.round.sd(<2 x double> %{{.*}}, <2 x double> %{{.*}}, i32 1)
+ // CHECK: extractelement
+ // CHECK: @llvm.floor.f64
+ // CHECK: insertelement
return _mm_floor_sd(x, y);
}
__m128 test_mm_floor_ss(__m128 x, __m128 y) {
// CHECK-LABEL: test_mm_floor_ss
- // CHECK: call <4 x float> @llvm.x86.sse41.round.ss(<4 x float> %{{.*}}, <4 x float> %{{.*}}, i32 1)
+ // CHECK: extractelement
+ // CHECK: @llvm.floor.f32
+ // CHECK: insertelement
return _mm_floor_ss(x, y);
}
Index: clang/test/CodeGen/avx512f-builtins.c
===
--- clang/test/CodeGen/avx512f-builtins.c
+++ clang/test/CodeGen/avx512f-builtins.c
@@ -7565,46 +7565,98 @@
return _mm512_min_round_ps(__A,__B,_MM_FROUND_CUR_DIRECTION);
}
+__m512 test_mm512_floor_ps(__m512 __A)
+{
+ // CHECK-LABEL: @test_mm512_floor_ps
+ // CHECK: @llvm.floor.v16f32
+ // CHECK-NOT: select
+ return _mm512_floor_ps(__A);
+}
+
+__m512d test_mm512_floor_pd(__m512d __A)
+{
+ // CHECK-LABEL: @test_mm512_floor_pd
+ // CHECK: @llvm.floor.v8f64
+ // CHECK-NOT: select
+ return _mm512_floor_pd(__A);
+}
+
__m512 test_mm512_mask_floor_ps (__m512 __W, __mmask16 __U, __m512 __A)
{
- // CHECK-LABEL: @test_mm512_mask_floor_ps
- // CHECK: @llvm.x86.avx512.mask.rndscale.ps.512
+ // CHECK-LABEL: @test_mm512_mask_floor_ps
+ // CHECK: @llvm.floor.v16f32
+ // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> %{{.*}}
return _mm512_mask_floor_ps (__W,__U,__A);
}
__m512d test_mm512_mask_floor_pd (__m512d __W, __mmask8 __U, __m512d __A)
{
- // CHECK-LABEL: @test_mm512_mask_floor_pd
- // CHECK: @llvm.x86.avx512.mask.rndscale.pd.512
+ // CHECK-LABEL: @test_mm512_mask_floor_pd
+ // CHECK: @llvm.floor.v8f64
+ // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
return _mm512_mask_floor_pd (__W,__U,__A);
}
+__m512 test_mm512_ceil_ps(__m512 __A)
+{
+ // CHECK-LABEL: @test_mm512_ceil_ps
+ // CHECK: @llvm.ceil.v16f32
+ // CHECK-NOT: select
+ return _mm512_ceil_ps(__A);
+}
+
+__m512d test_mm512_ceil_pd(__m512d __A)
+{
+ // CHECK-LABEL: @test_mm512_ceil_pd
+ // CHECK: @llvm.ceil.v8f64
+ // CHECK-NOT: select
+ return _mm512_ceil_pd(__A);
+}
+
__m512 test_mm512_mask_ceil_ps (__m512 __W, __mmask16 __U, __m512 __A)
{
- // CHECK-LABEL: @test_mm512_mask_ceil_ps
- // CHECK: @llvm.x86.avx512.mask.rndscale.ps.512
+ // CHECK-LABEL: @test_mm512_mask_ceil_ps
+ // CHECK: @llvm.ceil.v16f32
+ // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> %{{.*}}
return _mm512_mask_ceil_ps (__W,__U,__A);
}
[PATCH] D47643: Rewrite JSON dispatcher loop using C IO (FILE*) instead of std::istream.
sammccall created this revision.
sammccall added reviewers: malaperle, ilya-biryukov.
Herald added subscribers: cfe-commits, jkorous, ioeric, klimek.
The EINTR loop around getline was added to fix an issue with mac gdb, but seems
to loop infinitely in rare cases on linux where the parent editor exits (most
reports with VSCode).
I can't work out how to fix this in a portable way with std::istream, but the
C APIs have clearer contracts and LLVM has a RetryAfterSignal function for use
with them which seems battle-tested.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47643
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/tool/ClangdMain.cpp
test/clangd/too_large.test
Index: test/clangd/too_large.test
===
--- test/clangd/too_large.test
+++ test/clangd/too_large.test
@@ -4,4 +4,4 @@
#
Content-Length: 2147483648
-# STDERR: Skipped overly large message
+# STDERR: Refusing to read message
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -238,5 +238,5 @@
llvm::set_thread_name("clangd.main");
// Change stdin to binary to not lose \r\n on windows.
llvm::sys::ChangeStdinToBinary();
- return LSPServer.run(std::cin, InputStyle) ? 0 : NoShutdownRequestErrorCode;
+ return LSPServer.run(stdin, InputStyle) ? 0 : NoShutdownRequestErrorCode;
}
Index: clangd/JSONRPCDispatcher.h
===
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -102,7 +102,9 @@
/// if it is.
/// Input stream(\p In) must be opened in binary mode to avoid preliminary
/// replacements of \r\n with \n.
-void runLanguageServerLoop(std::istream &In, JSONOutput &Out,
+/// We use C-style FILE* for reading as std::istream has unclear interaction
+/// with signals, which are sent by debuggers on some OSs.
+void runLanguageServerLoop(std::FILE *In, JSONOutput &Out,
JSONStreamStyle InputStyle,
JSONRPCDispatcher &Dispatcher, bool &IsDone);
Index: clangd/JSONRPCDispatcher.cpp
===
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -14,6 +14,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Chrono.h"
+#include "llvm/Support/Errno.h"
#include "llvm/Support/SourceMgr.h"
#include
@@ -66,7 +67,7 @@
Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S;
Outs.flush();
}
- log(llvm::Twine("--> ") + S);
+ log(llvm::Twine("--> ") + S + "\n");
}
void JSONOutput::log(const Twine &Message) {
@@ -180,47 +181,54 @@
return true;
}
-static llvm::Optional readStandardMessage(std::istream &In,
+// Tries to read a line up to and including \n.
+// If failing, feof() or ferror() will be set.
+static bool readLine(std::FILE *In, std::string &Out) {
+ static constexpr int BufSize = 1024;
+ size_t Size = 0;
+ Out.clear();
+ for (;;) {
+Out.resize(Size + BufSize);
+// Handle EINTR which is sent when a debugger attaches on some platforms.
+if (!llvm::sys::RetryAfterSignal(nullptr, ::fgets, &Out[Size], BufSize, In))
+ return false;
+size_t Read = std::strlen(&Out[Size]);
+if (Read < BufSize - 1) {
+ Out.resize(Size + Read);
+ return true;
+}
+Size += Read;
+ }
+}
+
+// Returns None when:
+// - ferror() or feof() are set.
+// - Content-Length is missing or empty (protocol error)
+static llvm::Optional readStandardMessage(std::FILE *In,
JSONOutput &Out) {
// A Language Server Protocol message starts with a set of HTTP headers,
// delimited by \r\n, and terminated by an empty line (\r\n).
unsigned long long ContentLength = 0;
- while (In.good()) {
-std::string Line;
-std::getline(In, Line);
-if (!In.good() && errno == EINTR) {
- In.clear();
- continue;
-}
+ std::string Line;
+ while (true) {
+if (feof(In) || ferror(In) || !readLine(In, Line))
+ return llvm::None;
Out.mirrorInput(Line);
-// Mirror '\n' that gets consumed by std::getline, but is not included in
-// the resulting Line.
-// Note that '\r' is part of Line, so we don't need to mirror it
-// separately.
-if (!In.eof())
- Out.mirrorInput("\n");
-
llvm::StringRef LineRef(Line);
// We allow comments in headers. Technically this isn't part
// of the LSP specification, but makes writing tests easier.
if (LineRef.startswith("#"))
continue;
-// Content-Type is a specified header, but does nothing.
-// Content-Length is a mandatory header. It specifies the length of the
-// following JSON.
-// It is unspecified what sequen
[PATCH] D47643: Rewrite JSON dispatcher loop using C IO (FILE*) instead of std::istream.
sammccall added a comment. @malaperle: would you mind patching this in and checking whether attaching a debugger still works on Mac (this was the reason for the EINTR loop, right?) I want to preserve this but now people other than me are complaining about old clangds hanging around and eating all their CPU :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework
juliehockett updated this revision to Diff 149500.
juliehockett marked 8 inline comments as done.
https://reviews.llvm.org/D43341
Files:
clang-doc/BitcodeReader.cpp
clang-doc/BitcodeReader.h
clang-doc/BitcodeWriter.cpp
clang-doc/BitcodeWriter.h
clang-doc/CMakeLists.txt
clang-doc/Representation.cpp
clang-doc/Representation.h
clang-doc/tool/ClangDocMain.cpp
test/clang-doc/bc-comment.cpp
test/clang-doc/bc-namespace.cpp
test/clang-doc/bc-record.cpp
Index: test/clang-doc/bc-record.cpp
===
--- /dev/null
+++ test/clang-doc/bc-record.cpp
@@ -0,0 +1,254 @@
+// This test requires Linux due to the system-dependent USR for the
+// inner class in function H.
+// REQUIRES: system-linux
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump-intermediate -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/bc/ACE81AFA6627B4CEF2B456FB6E1252925674AF7E.bc --dump | FileCheck %s --check-prefix CHECK-A
+// RUN: llvm-bcanalyzer %t/docs/bc/FC07BD34D5E77782C263FA97929EA8753740.bc --dump | FileCheck %s --check-prefix CHECK-B
+// RUN: llvm-bcanalyzer %t/docs/bc/1E3438A08BA22025C0B46289FF0686F92C8924C5.bc --dump | FileCheck %s --check-prefix CHECK-BC
+// RUN: llvm-bcanalyzer %t/docs/bc/06B5F6A19BA9F6A832E127C9968282B94619B210.bc --dump | FileCheck %s --check-prefix CHECK-C
+// RUN: llvm-bcanalyzer %t/docs/bc/0921737541208B8FA9BB42B60F78AC1D779AA054.bc --dump | FileCheck %s --check-prefix CHECK-D
+// RUN: llvm-bcanalyzer %t/docs/bc/289584A8E0FF4178A794622A547AA622503967A1.bc --dump | FileCheck %s --check-prefix CHECK-E
+// RUN: llvm-bcanalyzer %t/docs/bc/DEB4AC1CD9253CD9EF7FBE6BCAC506D77984ABD4.bc --dump | FileCheck %s --check-prefix CHECK-ECON
+// RUN: llvm-bcanalyzer %t/docs/bc/BD2BDEBD423F80BACCEA75DE6D6622D355FC2D17.bc --dump | FileCheck %s --check-prefix CHECK-EDES
+// RUN: llvm-bcanalyzer %t/docs/bc/E3B54702FABFF4037025BA194FC27C47006330B5.bc --dump | FileCheck %s --check-prefix CHECK-F
+// RUN: llvm-bcanalyzer %t/docs/bc/B6AC4C5C9F2EA3F2B3ECE1A33D349F4EE502B24E.bc --dump | FileCheck %s --check-prefix CHECK-H
+// RUN: llvm-bcanalyzer %t/docs/bc/6BA1EE2B3DAEACF6E4306F10AF44908F4807927C.bc --dump | FileCheck %s --check-prefix CHECK-I
+// RUN: llvm-bcanalyzer %t/docs/bc/5093D428CDC62096A67547BA52566E4FB9404EEE.bc --dump | FileCheck %s --check-prefix CHECK-PM
+// RUN: llvm-bcanalyzer %t/docs/bc/CA7C7935730B5EACD25F080E9C83FA087CCDC75E.bc --dump | FileCheck %s --check-prefix CHECK-X
+// RUN: llvm-bcanalyzer %t/docs/bc/641AB4A3D36399954ACDE29C7A8833032BF40472.bc --dump | FileCheck %s --check-prefix CHECK-Y
+
+void H() {
+ class I {};
+}
+// CHECK-H:
+ // CHECK-H-NEXT:
+ // CHECK-H-NEXT: blob data = 'H'
+ // CHECK-H-NEXT: blob data = '{{.*}}'
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+ // CHECK-H-NEXT: blob data = 'void'
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+
+
+// CHECK-I:
+ // CHECK-I-NEXT:
+ // CHECK-I-NEXT: blob data = 'I'
+ // CHECK-I-NEXT:
+// CHECK-I-NEXT:
+// CHECK-I-NEXT: blob data = 'H'
+// CHECK-I-NEXT:
+// CHECK-I-NEXT:
+ // CHECK-I-NEXT:
+ // CHECK-I-NEXT: blob data = '{{.*}}'
+ // CHECK-I-NEXT:
+// CHECK-I-NEXT:
+
+union A { int X; int Y; };
+// CHECK-A:
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'A'
+ // CHECK-A-NEXT: blob data = '{{.*}}'
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'int'
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+// CHECK-A-NEXT: blob data = 'X'
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'int'
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+// CHECK-A-NEXT: blob data = 'Y'
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+
+enum B { X, Y };
+// CHECK-B:
+ // CHECK-B-NEXT:
+ // CHECK-B-NEXT: blob data = 'B'
+ // CHECK-B-NEXT: blob data = '{{.*}}'
+ // CHECK-B-NEXT: blob data = 'X'
+ // CHECK-B-NEXT: blob data = 'Y'
+// CHECK-B-NEXT:
+
+enum class Bc { A, B };
+// CHECK-BC:
+ // CHECK-BC-NEXT:
+ // CHECK-BC-NEXT: blob data = 'Bc'
+ // CHECK-BC-NEXT: blob data = '{{.*}}'
+ // CHECK-BC-NEXT:
+ // CHECK-BC-NEXT: blob data = 'A'
+ // CHECK-BC-NEXT: blob data = 'B'
+// CHECK-BC-NEXT:
+
+struct C { int i; };
+// CHECK-C:
+ // CHECK-C-NEXT:
+ // CHECK-C-NEXT: blob data = 'C'
+ // CHECK-C-NEXT: blob data = '{{.*}}'
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+ // CHECK-C-NEXT: blob data = 'int'
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+// CHECK-C-NEXT: blob data = 'i'
+// CHECK-C-NEXT:
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+
+class D {};
+// CHECK-D:
+ // CHECK-D-NEXT:
+ // CHECK-D-NEXT: blob data = 'D'
+ // CHECK-D-NEXT: blob data = '{{.*}}'
+ // CHECK-D-NEXT:
+// CHECK-D-NEX
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. This looks OK, the asymmetry still seems a little odd to me and could be reduced a little. (Please also resolve Ilya's question about references with him, I don't have a strong opinion) Comment at: clangd/index/SymbolCollector.cpp:303 + // canonical declaration in the index. + if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) +D = ASTNode.OrigD; Ah, now I understand - we're picking another to use as canonical. But the exception for definitions should apply here too. And there's nothing special about *OrigD* that makes it a good pick for canonical. For determinism, can we instead iterate over the redecls of D and pick the first one that's not a friend or is a definition? (I'd pull that check out into a function `shouldSkipDecl` and rename the existing one `shouldSkipSymbol`, but up to you) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
Meinersbur added a comment. The RFC: https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333775 - [Coverage] Remove a test dependency on the itanium ABI
Author: vedantk
Date: Fri Jun 1 10:11:18 2018
New Revision: 333775
URL: http://llvm.org/viewvc/llvm-project?rev=333775&view=rev
Log:
[Coverage] Remove a test dependency on the itanium ABI
This should address a bot failure:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/9994/
Modified:
cfe/trunk/test/CoverageMapping/label.cpp
Modified: cfe/trunk/test/CoverageMapping/label.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/label.cpp?rev=333775&r1=333774&r2=333775&view=diff
==
--- cfe/trunk/test/CoverageMapping/label.cpp (original)
+++ cfe/trunk/test/CoverageMapping/label.cpp Fri Jun 1 10:11:18 2018
@@ -48,7 +48,17 @@ b: // CHECK-NE
x = x + 1;
}
- // CHECK-NEXT: main
+// CHECK-NEXT: test3
+#define a b
+void test3() {
+ if (0)
+goto b; // CHECK: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:1 =
[[retnCount:#[0-9]+]]
+a: // CHECK-NEXT: Expansion,File 0, [[@LINE]]:1 -> [[@LINE]]:2 = [[retnCount]]
(Expanded file = 1)
+ return; // CHECK-NEXT: File 0, [[@LINE-1]]:2 -> [[@LINE]]:9 = [[retnCount]]
+}
+#undef a
+
+ // CHECK: main
int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 ->
{{[0-9]+}}:2 = #0
int j = 0;
for(int i = 0; i < 10; ++i) { // CHECK: File 0, [[@LINE]]:31 ->
[[@LINE+13]]:4 = #1
@@ -69,12 +79,3 @@ int main() { // CHECK-NE
test1(0);
test2(2);
}
-
-// CHECK-LABEL: _Z5test3v:
-#define a b
-void test3() {
- if (0)
-goto b; // CHECK: Gap,File 0, [[@LINE]]:11 -> [[@LINE+1]]:1 =
[[retnCount:#[0-9]+]]
-a: // CHECK-NEXT: Expansion,File 0, [[@LINE]]:1 -> [[@LINE]]:2 = [[retnCount]]
(Expanded file = 1)
- return; // CHECK-NEXT: File 0, [[@LINE-1]]:2 -> [[@LINE]]:9 = [[retnCount]]
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47578: Do not enforce absolute path argv0 in windows
ruiu added a comment.
It seems like you are trying a bit too hard to keep the original code which is
not always a good idea to keep the clarity of the code.
So, IIUC, you this function:
- returns a full filename of the current executable. That can be written using
GetModuleFileName and GetLongPathName
then you basically do the following to
sys::path::remove_filename(Arg0);
sys::path::append(Arg0, sys::path::filename(ExectuablePath));
Comment at: llvm/lib/Support/Windows/Process.inc:211
static std::error_code ExpandShortFileName(const wchar_t *Arg,
+ SmallVectorImpl &LongPath)
{
This function is used only by your new function, so it is probably better to
inline it.
Comment at: llvm/lib/Support/Windows/Process.inc:226
+
+static std::error_code GetLongArgv0FullPath(const wchar_t *Argv0,
+SmallVectorImpl
&LongArgv0) {
You can always ignore Argv0, no? Since GetModuleFileName is always available,
you probably should use it unconditionally, ignoring the original argv[0].
Comment at: llvm/lib/Support/Windows/Process.inc:251
+ // This may change argv0 like below,
+ // * clang -> clang.exe (just add extension)
+ // * CLANG_~1.EXE -> clang++.exe (extend shorname)
I believe GetModuleFileName always returns a path with an extension, though it
might be 8.3 path.
Comment at: llvm/lib/Support/Windows/Process.inc:268-269
+
+ return windows::UTF8ToUTF16(StringRef(UTF8Argv0.data(), UTF8Argv0.size()),
+ LongArgv0);
}
Don't convert utf-8 to utf-16 only to convert it back to utf-8 immediately
after returning from this function.
https://reviews.llvm.org/D47578
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
tra added a comment. IIUIC, nv_weak is a synonym for weak (why, oh why did they need it?) You may need to hunt down and change few other places that deal with the weak attribute. E.g.: https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47070: [CUDA] Upgrade linked bitcode to enable inlining
tra added a comment. IMO overriding TargetTransformInfo::areInlineCompatible to always return true on NVPTX is what we want to do instead of upgrading everything else. AFAICT, on NVPTX there's no reason to prevent inlining due to those attributes -- we'll never generate code, nor will we ever execute it on any other GPU than we're currently compiling for. This should get you going until I figure out how to have target-specific builtins without sticking target-cpu and target-features attributes on everything. Repository: rC Clang https://reviews.llvm.org/D47070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. LGTM with a nit on a test name. Comment at: test/Analysis/pr37646.c:1 +// REQUIRES: z3 +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -analyzer-checker=core -analyzer-store=region -analyzer-constraints=z3 -verify %s The tests are already quite messy, but adding a new file per each bug seems excessive. Could we take your test and `test/Analysis/apsint.c` and combine them into e.g. `z3_apsint_encoding.c` ? (also adding a link to bugzilla) Repository: rC Clang https://reviews.llvm.org/D47617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)
george.karpenkov added a comment. Thanks! Repository: rC Clang https://reviews.llvm.org/D47617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
aaron.ballman added a comment. In https://reviews.llvm.org/D47201#1119249, @tra wrote: > IIUIC, nv_weak is a synonym for weak (why, oh why did they need > it?) > You may need to hunt down and change few other places that deal with the > weak attribute. > E.g.: > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 If it is truly a synonym for weak, then a better implementation would be to make no semantic distinction between the two attributes -- just add new spellings to weak. If you need to make minor distinctions between the spellings, you can do it using accessors on the attribute. Comment at: include/clang/Basic/Attr.td:1515 let LangOpts = [CUDA]; + let Documentation = [Undocumented]; } No new, undocumented attributes, please. Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)
NoQ added a comment. We might as well make a directory for z3-specific tests. Eg., `z3/bool-bit-width.c`. Also does this test need to be z3-specific? We would also not like to crash here without z3. Repository: rC Clang https://reviews.llvm.org/D47617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
ahatanak added a comment. In https://reviews.llvm.org/D45015#1105388, @rsmith wrote: > In https://reviews.llvm.org/D45015#1105372, @vsapsai wrote: > > > What when compiler has `__builtin_operator_new`, > > `__builtin_operator_delete`? If I build libc++ tests with recent Clang > > which has these builtins and run tests with libc++.dylib from old Darwin, > > there are no linkage errors. Should we define `__cpp_aligned_allocation` in > > this case? > > > I don't see why that should make any difference -- those builtins call the > same functions that the new and delete operator call. Perhaps libc++ isn't > calling the forms of those builtins that take an alignment argument yet? It looks like clang currently doesn't issue a warning when a call to __builtin_operator_new or __builtin_operator_delete calls an aligned allocation function that is not support by the OS version. I suppose we should fix this? // no warning issued when triple is "thumbv7-apple-ios5.0.0" even though aligned allocation is unavailable. void *p = __builtin_operator_new(100, std::align_val_t(32)); Repository: rC Clang https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
There's quite a lot of code duplication here, I think we could do better with
that. Great job modeling semantics though!
Comment at: lib/Analysis/CFG.cpp:1320
+auto *MTE = cast(Child);
+findConstructionContexts(ConstructionContextLayer::create(
+ cfg->getBumpVectorContext(), MTE, Layer),
There are three repeated calls to `findConstructionContexts`, with only the
second argument changing. Perhaps a helper lambda closure would help?
Comment at: lib/Analysis/CFG.cpp:4959
+ const Stmt *S1 = nullptr, *S2 = nullptr, *S3 = nullptr;
+ const ConstructionContext *CC1 = nullptr;
switch (CC->getKind()) {
begs for a comment differentiating CC from CC1
Comment at: lib/Analysis/CFG.cpp:5014
}
if (S1) {
OS << ", ";
three blocks below could really benefit from a helper function.
Comment at: lib/Analysis/ConstructionContext.cpp:140
- assert(TopLayer->isLast());
- return create(C, nullptr, MTE);
+ // Handle pre-C++17 copy and move elision.
+ const CXXConstructExpr *ElidedCE = nullptr;
seems this chunk has a fair bit of code duplication vs. lines 79-99
Repository:
rC Clang
https://reviews.llvm.org/D47616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45517: [analyzer] False positive refutation with Z3
george.karpenkov added a comment. @mikhail.ramalho I assume you know it, but just in case, you can mark dependencies in phabricator by adding "parent" revisions. https://reviews.llvm.org/D45517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.
akyrtzi added a comment. We could leave `disableSourceFileDiagnostics` off until someone finds a use case for it. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r333776 - Mark __c11_atomic_load as const
Author: jfb
Date: Fri Jun 1 11:02:53 2018
New Revision: 333776
URL: http://llvm.org/viewvc/llvm-project?rev=333776&view=rev
Log:
Mark __c11_atomic_load as const
Summary:
C++11 onwards specs the non-member functions atomic_load and
atomic_load_explicit as taking the atomic by const (potentially volatile)
pointer. C11, in its infinite wisdom, decided to drop the const, and C17 will
fix this with DR459 (the current draft forgot to fix B.16, but that’s not the
normative part).
This patch fixes the libc++ version of the __c11_atomic_load builtins defined
for GCC's compatibility sake.
D47618 takes care of the clang side.
Discussion: http://lists.llvm.org/pipermail/cfe-dev/2018-May/058129.html
Reviewers: EricWF, mclow.lists
Subscribers: christof, cfe-commits
Differential Revision: https://reviews.llvm.org/D47613
Modified:
libcxx/trunk/include/atomic
Modified: libcxx/trunk/include/atomic
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/atomic?rev=333776&r1=333775&r2=333776&view=diff
==
--- libcxx/trunk/include/atomic (original)
+++ libcxx/trunk/include/atomic Fri Jun 1 11:02:53 2018
@@ -698,7 +698,7 @@ static inline void __c11_atomic_store(_A
}
template
-static inline _Tp __c11_atomic_load(volatile _Atomic(_Tp)* __a,
+static inline _Tp __c11_atomic_load(const volatile _Atomic(_Tp)* __a,
memory_order __order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
@@ -707,7 +707,7 @@ static inline _Tp __c11_atomic_load(vola
}
template
-static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order) {
+static inline _Tp __c11_atomic_load(const _Atomic(_Tp)* __a, memory_order
__order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47613: Mark __c11_atomic_load as const
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333776: Mark __c11_atomic_load as const (authored by jfb,
committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47613
Files:
libcxx/trunk/include/atomic
Index: libcxx/trunk/include/atomic
===
--- libcxx/trunk/include/atomic
+++ libcxx/trunk/include/atomic
@@ -698,16 +698,16 @@
}
template
-static inline _Tp __c11_atomic_load(volatile _Atomic(_Tp)* __a,
+static inline _Tp __c11_atomic_load(const volatile _Atomic(_Tp)* __a,
memory_order __order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
-static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order) {
+static inline _Tp __c11_atomic_load(const _Atomic(_Tp)* __a, memory_order
__order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
Index: libcxx/trunk/include/atomic
===
--- libcxx/trunk/include/atomic
+++ libcxx/trunk/include/atomic
@@ -698,16 +698,16 @@
}
template
-static inline _Tp __c11_atomic_load(volatile _Atomic(_Tp)* __a,
+static inline _Tp __c11_atomic_load(const volatile _Atomic(_Tp)* __a,
memory_order __order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
-static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order) {
+static inline _Tp __c11_atomic_load(const _Atomic(_Tp)* __a, memory_order __order) {
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333778 - [X86] Rewrite avx512vbmi unmasked and maskz macro intrinsics to be wrappers around their __builtin function with appropriate arguments rather than just passing arguments to the masked intrin
Author: ctopper Date: Fri Jun 1 11:26:35 2018 New Revision: 333778 URL: http://llvm.org/viewvc/llvm-project?rev=333778&view=rev Log: [X86] Rewrite avx512vbmi unmasked and maskz macro intrinsics to be wrappers around their __builtin function with appropriate arguments rather than just passing arguments to the masked intrinsic. This is more consistent with all of our other avx512 macro intrinsics. It also fixes a bad cast where an argument was casted to mmask8 when it should have been a mmask16. Modified: cfe/trunk/lib/Headers/avx512vbmi2intrin.h cfe/trunk/lib/Headers/avx512vlvbmi2intrin.h Modified: cfe/trunk/lib/Headers/avx512vbmi2intrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vbmi2intrin.h?rev=333778&r1=333777&r2=333778&view=diff == --- cfe/trunk/lib/Headers/avx512vbmi2intrin.h (original) +++ cfe/trunk/lib/Headers/avx512vbmi2intrin.h Fri Jun 1 11:26:35 2018 @@ -150,10 +150,18 @@ _mm512_maskz_expandloadu_epi8(__mmask64 (__mmask8)(U)) #define _mm512_maskz_shldi_epi64(U, A, B, I) \ - _mm512_mask_shldi_epi64(_mm512_setzero_si512(), (U), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldq512_mask((__v8di)(__m512i)(A), \ + (__v8di)(__m512i)(B), \ + (int)(I), \ + (__v8di)_mm512_setzero_si512(), \ + (__mmask8)(U)) #define _mm512_shldi_epi64(A, B, I) \ - _mm512_mask_shldi_epi64(_mm512_undefined(), (__mmask8)(-1), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldq512_mask((__v8di)(__m512i)(A), \ + (__v8di)(__m512i)(B), \ + (int)(I), \ + (__v8di)_mm512_undefined_epi32(), \ + (__mmask8)-1) #define _mm512_mask_shldi_epi32(S, U, A, B, I) \ (__m512i)__builtin_ia32_vpshldd512_mask((__v16si)(__m512i)(A), \ @@ -163,10 +171,18 @@ _mm512_maskz_expandloadu_epi8(__mmask64 (__mmask16)(U)) #define _mm512_maskz_shldi_epi32(U, A, B, I) \ - _mm512_mask_shldi_epi32(_mm512_setzero_si512(), (U), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldd512_mask((__v16si)(__m512i)(A), \ + (__v16si)(__m512i)(B), \ + (int)(I), \ + (__v16si)_mm512_setzero_si512(), \ + (__mmask16)(U)) #define _mm512_shldi_epi32(A, B, I) \ - _mm512_mask_shldi_epi32(_mm512_undefined(), (__mmask16)(-1), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldd512_mask((__v16si)(__m512i)(A), \ + (__v16si)(__m512i)(B), \ + (int)(I), \ + (__v16si)_mm512_undefined_epi32(), \ + (__mmask16)-1) #define _mm512_mask_shldi_epi16(S, U, A, B, I) \ (__m512i)__builtin_ia32_vpshldw512_mask((__v32hi)(__m512i)(A), \ @@ -176,10 +192,18 @@ _mm512_maskz_expandloadu_epi8(__mmask64 (__mmask32)(U)) #define _mm512_maskz_shldi_epi16(U, A, B, I) \ - _mm512_mask_shldi_epi16(_mm512_setzero_si512(), (U), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldw512_mask((__v32hi)(__m512i)(A), \ + (__v32hi)(__m512i)(B), \ + (int)(I), \ + (__v32hi)_mm512_setzero_si512(), \ + (__mmask32)(U)) #define _mm512_shldi_epi16(A, B, I) \ - _mm512_mask_shldi_epi16(_mm512_undefined(), (__mmask32)(-1), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshldw512_mask((__v32hi)(__m512i)(A), \ + (__v32hi)(__m512i)(B), \ + (int)(I), \ + (__v32hi)_mm512_undefined_epi32(), \ + (__mmask32)-1) #define _mm512_mask_shrdi_epi64(S, U, A, B, I) \ (__m512i)__builtin_ia32_vpshrdq512_mask((__v8di)(__m512i)(A), \ @@ -189,10 +213,18 @@ _mm512_maskz_expandloadu_epi8(__mmask64 (__mmask8)(U)) #define _mm512_maskz_shrdi_epi64(U, A, B, I) \ - _mm512_mask_shrdi_epi64(_mm512_setzero_si512(), (U), (A), (B), (I)) + (__m512i)__builtin_ia32_vpshrdq512_mask((__v8di)(__m512i)(A), \ + (__v8di)(__m512i)(B), \ + (int)(I), \ + (__v8di)_mm512_setzero_si512(), \ + (__mmask8)(U)) #define _mm512_shrdi_epi64(A,
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric updated this revision to Diff 149526.
ioeric added a comment.
- Make canonical decls determinstic.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,50 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
+TEST_F(SymbolCollectorTest, ReferencesInFriendDecl) {
+ const std::string Header = R"(
+class X;
+class Y;
+ )";
+ const std::string Main = R"(
+class C {
+ friend ::X;
+ friend class Y;
+};
+ )";
+ CollectorOpts.CountReferences = true;
+ runSymbolCollector(Header, Main);
+ for (const auto &Sym : Symbols)
+llvm::errs() << Sym.Name << ": " << Sym.References << "\n";
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), Refs(1)),
+AllOf(QName("Y"), Refs(1;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -80,6 +80,12 @@
Options Opts;
// Decls referenced from the current TU, flushed on finish().
llvm::DenseSet ReferencedDecls;
+ // Maps canonical declaration provided by clang to canonical declaration for
+ // an index symbol, if clangd prefers a different declaration than that
+ // provided by clang. For example, friend declaration might be considered
+ // canonical by clang but should not be considered canonical in the index
+ // unless it's a definition.
+ llvm::DenseMap CanonicalDecls;
};
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,23 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ assert(ASTNode.OrigD);
+ // If OrigD is an declaration associated with a friend declaration and it's
+ // not a definition, skip it. Note that OrigD is the occurrence that the
+ // collector is currently visiting.
+ if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+ !(Roles & static_cast(index::SymbolRole::Definition)))
+return true;
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index. If D is a defintion and is not OrigD,
+ // D would have been picked as the canonical declaration, if it has been
+ // visited, or OrigD, which could only be a non-friend declaration as
multiple
+ // definitons are not possible, will be preferred.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
+// Either use a OrigD or a previous recorded canonical declaration as the
+// canonical declaration.
+D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second;
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,50 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedE
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. Hi all, I'll be attempting to commit this patch around 6pm PT today unless anyone has any more comments on this specific patch. Any other suggestions regarding potential design changes can be discussed in future patches since this is only the first of many. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric updated this revision to Diff 149528.
ioeric added a comment.
- Remove debug message.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,48 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Header.range("y"))),
+ AllOf(QName("nx::Z"), DeclRange(Header.range("z"))),
+ AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))),
+ AllOf(QName("nx::bar"), DeclRange(Header.range("bar");
+}
+
+TEST_F(SymbolCollectorTest, ReferencesInFriendDecl) {
+ const std::string Header = R"(
+class X;
+class Y;
+ )";
+ const std::string Main = R"(
+class C {
+ friend ::X;
+ friend class Y;
+};
+ )";
+ CollectorOpts.CountReferences = true;
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), Refs(1)),
+AllOf(QName("Y"), Refs(1;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -80,6 +80,12 @@
Options Opts;
// Decls referenced from the current TU, flushed on finish().
llvm::DenseSet ReferencedDecls;
+ // Maps canonical declaration provided by clang to canonical declaration for
+ // an index symbol, if clangd prefers a different declaration than that
+ // provided by clang. For example, friend declaration might be considered
+ // canonical by clang but should not be considered canonical in the index
+ // unless it's a definition.
+ llvm::DenseMap CanonicalDecls;
};
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -290,6 +290,23 @@
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
+ assert(ASTNode.OrigD);
+ // If OrigD is an declaration associated with a friend declaration and it's
+ // not a definition, skip it. Note that OrigD is the occurrence that the
+ // collector is currently visiting.
+ if ((ASTNode.OrigD->getFriendObjectKind() !=
+ Decl::FriendObjectKind::FOK_None) &&
+ !(Roles & static_cast(index::SymbolRole::Definition)))
+return true;
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index. If D is a defintion and is not OrigD,
+ // D would have been picked as the canonical declaration, if it has been
+ // visited, or OrigD, which could only be a non-friend declaration as
multiple
+ // definitons are not possible, will be preferred.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
+// Either use a OrigD or a previous recorded canonical declaration as the
+// canonical declaration.
+D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second;
const NamedDecl *ND = llvm::dyn_cast(D);
if (!ND)
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -812,6 +812,48 @@
QName("nx::Kind"), QName("nx::Kind_Fine")));
}
+TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) {
+ Annotations Header(R"(
+namespace nx {
+ class $z[[Z]] {};
+ class X {
+friend class Y;
+friend class Z;
+friend void foo();
+friend void $bar[[bar]]() {}
+ };
+ class $y[[Y]] {};
+ void $foo[[foo]]();
+}
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ QName("nx"), QName("nx::X"),
+ AllOf(QName("nx::Y"), DeclRange(Head
[PATCH] D45517: [analyzer] False positive refutation with Z3
mikhail.ramalho updated this revision to Diff 149524.
mikhail.ramalho added a comment.
- Simplified the API even further by constructing a Z3ConstraintManager object
directly.
- Update isModelFeasible to return a isModelFeasible
- Update code with the fix for 1-bit long integer
https://reviews.llvm.org/D45517
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/Z3ConstraintManager.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
test/Analysis/z3-crosscheck.c
Index: test/Analysis/z3-crosscheck.c
===
--- /dev/null
+++ test/Analysis/z3-crosscheck.c
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -DNO_CROSSCHECK -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+
+#ifndef NO_CROSSCHECK
+// expected-no-diagnostics
+#endif
+
+int foo(int x)
+{
+ int *z = 0;
+ if ((x & 1) && ((x & 1) ^ 1))
+#ifdef NO_CROSSCHECK
+ return *z; // expected-warning {{Dereference of null pointer (loaded from variable 'z')}}
+#else
+ return *z; // no-warning
+#endif
+ return 0;
+}
+
+void g(int d);
+
+void f(int *a, int *b) {
+ int c = 5;
+ if ((a - b) == 0)
+c = 0;
+ if (a != b)
+#ifdef NO_CROSSCHECK
+g(3 / c); // expected-warning {{Division by zero}}
+#else
+g(3 / c); // no-warning
+#endif
+}
+
+_Bool nondet_bool();
+
+void h(int d) {
+ int x, y, k, z = 1;
+#ifdef NO_CROSSCHECK
+ while (z < k) { // expected-warning {{The right operand of '<' is a garbage value}}
+#else
+ // FIXME: Should warn about 'k' being a garbage value
+ while (z < k) { // no-warning
+#endif
+z = 2 * z;
+ }
+}
+
+void i() {
+ _Bool c = nondet_bool();
+ if (c) {
+h(1);
+ } else {
+h(2);
+ }
+}
+
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -8,7 +8,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/Z3ConstraintManager.h"
using namespace clang;
@@ -1013,6 +1013,50 @@
return State->set(CZ);
}
+void Z3ConstraintManager::addRangeConstraints(ConstraintRangeTy CR) {
+ for (const auto &I : CR) {
+SymbolRef Sym = I.first;
+
+Z3Expr Constraints = Z3Expr::fromBoolean(false);
+
+for (const auto &Range : I.second) {
+ const llvm::APSInt &From = Range.From();
+ const llvm::APSInt &To = Range.To();
+
+ QualType FromTy;
+ llvm::APSInt NewFromInt;
+ std::tie(NewFromInt, FromTy) = fixAPSInt(From);
+ Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt);
+
+ QualType SymTy;
+ Z3Expr Exp = getZ3Expr(Sym, &SymTy);
+ bool IsSignedTy = SymTy->isSignedIntegerOrEnumerationType();
+
+ QualType ToTy;
+ llvm::APSInt NewToInt;
+ std::tie(NewToInt, ToTy) = fixAPSInt(To);
+ Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt);
+ assert(FromTy == ToTy && "Range values have different types!");
+
+ Z3Expr LHS =
+ getZ3BinExpr(Exp, SymTy, BO_GE, FromExp, FromTy, /*RetTy=*/nullptr);
+ Z3Expr RHS =
+ getZ3BinExpr(Exp, SymTy, BO_LE, ToExp, FromTy, /*RetTy=*/nullptr);
+ Z3Expr SymRange = Z3Expr::fromBinOp(LHS, BO_LAnd, RHS, IsSignedTy);
+ Constraints =
+ Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy);
+}
+Solver.addConstraint(Constraints);
+ }
+}
+
+clang::ento::ConditionTruthVal Z3ConstraintManager::isModelFeasible() {
+ if(Solver.check() == Z3_L_FALSE)
+return false;
+
+ return ConditionTruthVal();
+}
+
//===--===//
// Internal implementation.
//===--===//
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -41,6 +41,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Cor
[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.
ioeric added a comment.
In https://reviews.llvm.org/D47623#1118951, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D47623#1118810, @sammccall wrote:
>
> > - friend decls that are not definitions should be ignored for indexing
> > purposes
>
>
> This is not generally true IIUC. A friend declaration can be a reference, a
> declaration or a definition.
>
> int foo(int);
> int bar(int, int);
> class ExistingFwdCls;
>
> class X {
> friend class ExistingFwdCls; // a reference and a declaration.
> friend class NewClass; // a new declaration.
> friend int foo(int); // a reference and a declaration.
> friend int baz(int, int, int); // a new declaration.
> };
>
> class Y {
> friend class ::ExistingFwdCls; // qualified => just a reference.
> friend int ::bar(int a, int b); // qualified => just a reference.
> friend int foo(int) { // a reference and a definition
> return 100;
> }
> };
>
>
> Note that friend functions with bodies are probably ok as canonical
> declaration, as they are often the only declaration, e.g.
>
> class Foo {
> friend bool operator < (Foo const& lhs, Foo const&lhs) {
> return false;
> }
> };
>
Sorry for the late response Ilya. I was trying to test these cases. So, with
the current change, if a real "canonical" declaration comes before the friend
decl, then the reference will still be recorded (I've added a test case for
this). Would this address your concern?
Comment at: clangd/index/SymbolCollector.cpp:303
+ // canonical declaration in the index.
+ if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
+D = ASTNode.OrigD;
sammccall wrote:
> Ah, now I understand - we're picking another to use as canonical. But the
> exception for definitions should apply here too. And there's nothing special
> about *OrigD* that makes it a good pick for canonical.
>
> For determinism, can we instead iterate over the redecls of D and pick the
> first one that's not a friend or is a definition?
>
> (I'd pull that check out into a function `shouldSkipDecl` and rename the
> existing one `shouldSkipSymbol`, but up to you)
> But the exception for definitions should apply here too
I'm not sure why this is needed. Do you have an example?
> And there's nothing special about *OrigD* that makes it a good pick for
> canonical.
I think `OrigD` is "special" because we have checked that it's not an
uninteresting friend decl.
> For determinism, can we instead iterate over the redecls of D and pick the
> first one that's not a friend or is a definition?
This seems to be the most ideal option, although, in practice, it's a bit hard
to check if each `redecl` is a definition, as we don't have `Roles` for them.
To make it more deterministic, I added a map from clang's canonical decl to
clangd's "canonical" decl so that we could be sure that all redecls would have
the same `D`. WDYT?
>(I'd pull that check out into a function shouldSkipDecl and rename the
>existing one shouldSkipSymbol, but up to you)
I gave this a try, but it couldn't find a very good abstraction. Happy to chat
more next week.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
