Re: [PATCH] D18700: [Inline asm][GCC compatibility] Handle %v-prefixed code in inline assembly
echristo added a comment.
Hi,
Couple of inline comments. The direction is getting there, but there's some
overall architecture issues to sort out I think.
Thanks!
-eric
Comment at: include/clang/AST/Stmt.h:1555
@@ +1554,3 @@
+ // target feature (by attribute or invocation options).
+ bool SupportsAVX;
+
I'd prefer not to keep SupportsAVX cached, or pass it though, but rather look
up what we need from the function if we need it. We can also cache the set of
valid subtarget features on the Stmt if necessary from the Function.
Comment at: lib/Sema/SemaStmtAsm.cpp:144-146
@@ +143,5 @@
+// find a function context.
+static void CollectTargetFeatures(Sema &S,
+ std::vector &TargetFeatures,
+ StringRef &TargetCPU) {
+ DeclContext *DC = S.CurContext;
Hate to say, but this is the point that we now have 3 of these scattered around
- can you take a look at what it will take to merge all of these in a way that
we can look up the cached features?
http://reviews.llvm.org/D18700
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18073: Add memory allocating functions
ariccio updated this revision to Diff 55774.
ariccio added a comment.
Only check for MSVC-specific functions when actually building FOR MSVC (i.e.
`isWindowsMSVCEnvironment`).
Sidenote: is there any reason why none of the `ASTContext&`s are `const`ified
in this file?
http://reviews.llvm.org/D18073
Files:
llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
llvm/tools/clang/test/Analysis/alternative-malloc-api.c
llvm/tools/clang/test/Analysis/malloc.c
Index: llvm/tools/clang/test/Analysis/alternative-malloc-api.c
===
--- llvm/tools/clang/test/Analysis/alternative-malloc-api.c
+++ llvm/tools/clang/test/Analysis/alternative-malloc-api.c
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+// Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines
+// _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use
+// the builtin type: "Using the typedef version can cause portability
+// problems", but we're ok here because we're not actually running anything.
+// Also of note is this cryptic warning: "The wchar_t type is not supported
+// when you compile C code".
+//
+// See the docs for more:
+// https://msdn.microsoft.com/en-us/library/dh8che7s.aspx
+#if !defined(_WCHAR_T_DEFINED)
+// "Microsoft implements wchar_t as a two-byte unsigned value"
+typedef unsigned short wchar_t;
+#define _WCHAR_T_DEFINED
+#endif // !defined(_WCHAR_T_DEFINED)
+
+void free(void *);
+
+char *tempnam(const char *dir, const char *pfx);
+
+void testTempnamLeak(const char* dir, const char* prefix) {
+ char* fileName = tempnam(dir, prefix);
+}// expected-warning {{Potential leak of memory pointed to by}}
+
+void testTempnamNoLeak(const char* dir, const char* prefix) {
+ char* fileName = tempnam(dir, prefix);
+ free(fileName);// no warning
+}
+
+
+// What does "ContentIsDefined" refer to?
+void testTempnamNoLeakContentIsDefined(const char* dir, const char* prefix) {
+ char* fileName = tempnam(dir, prefix);
+ char result = fileName[0];// no warning
+ free(fileName);
+}
+
+#if defined(_WIN32)
+char *_tempnam(const char *dir, const char *prefix);
+wchar_t *_wtempnam(const wchar_t *dir, const wchar_t *prefix);
+
+char *_tempnam_dbg(const char *dir, const char *prefix, int blockType,
+ const char *filename, int linenumber);
+
+wchar_t *_wtempnam_dbg(const wchar_t *dir, const wchar_t *prefix,
+ int blockType, const char *filename, int linenumber);
+
+void testWinTempnamLeak(const char* dir, const char* prefix) {
+ char* fileName = _tempnam(dir, prefix);
+}// expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinTempnamDbgLeak(const char* dir, const char* prefix) {
+ char* fileName = _tempnam_dbg(dir, prefix, 0, __FILE__, __LINE__);
+}// expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinWideTempnamLeak(const wchar_t* dir, const wchar_t* prefix) {
+ wchar_t* fileName = _wtempnam(dir, prefix);
+}// expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinWideTempnaDbgmLeak(const wchar_t* dir, const wchar_t* prefix) {
+ wchar_t* fileName = _wtempnam_dbg(dir, prefix, 0, __FILE__, __LINE__);
+}// expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinTempnamNoLeak(const char* dir, const char* prefix) {
+ char* fileName = _tempnam(dir, prefix);
+ free(fileName);// no warning
+}
+
+void testWinTempnamDbgNoLeak(const char* dir, const char* prefix) {
+ char* fileName = _tempnam_dbg(dir, prefix, 0, __FILE__, __LINE__);
+ free(fileName);// no warning
+}
+
+void testWinWideTempnamNoLeak(const wchar_t* dir, const wchar_t* prefix) {
+ wchar_t* fileName = _wtempnam(dir, prefix);
+ free(fileName);// no warning
+}
+
+void testWinWideTempnamDbgNoLeak(const wchar_t* dir, const wchar_t* prefix) {
+ wchar_t* fileName = _wtempnam_dbg(dir, prefix, 0, __FILE__, __LINE__);
+ free(fileName);// no warning
+}
+
+// What does "ContentIsDefined" refer to?
+void testWinTempnamNoLeakContentIsDefined(const char* dir, const char* prefix) {
+ char* fileName = _tempnam(dir, prefix);
+ char result = fileName[0];// no warning
+ free(fileName);
+}
+
+// What does "ContentIsDefined" refer to?
+void testWinWideTempnamNoLeakContentIsDefined(const wchar_t* dir, const wchar_t* prefix) {
+ wchar_t* fileName = _wtempnam(dir, prefix);
+ wchar_t result = fileName[0];// no warning
+ free(fileName);
+}
+#endif //defined(_WIN32)
Index: llvm/tools/clang/test/Analysis/malloc.c
===
--- llvm/tools/clang/test/Analysis/malloc.c
+++ llvm/tools/clang/test/Analysis/malloc.c
@@ -32,11 +32,37 @@
char *strndup(const char *s, size_t n);
int memcmp(const void *s1, const void *s2, size_t n);
+
// Wi
Re: [PATCH] D19783: Fix cv-qualification of '*this' captures (and nasty bug PR27507 introduced by commit 263921 "Implement Lambda Capture of *this by Value as [=, *this]")
faisalv added a comment.
Is it too soon for a *ping* ;)
p.s. just added some comments.
Comment at: lib/Sema/SemaExprCXX.cpp:904
@@ +903,3 @@
+ for (int I = FunctionScopes.size();
+ I-- && dyn_cast(FunctionScopes[I]);
+ IsFirstIteration = false,
That dyn_cast should be an 'isa'
Comment at: lib/Sema/SemaExprCXX.cpp:1062
@@ -964,1 +1061,3 @@
+ Expr *This =
+ new (Context) CXXThisExpr(Loc, AdjustedThisTy, /*isImplicit*/ true);
if (ByCopy) {
Hmm - I wonder if instead of using AdjustedThisTy, I should always use 'ThisTy'
since it is being used in the initializer expression, and should probably
inherit its cv qualifiers from an enclosing lambda? correct?
http://reviews.llvm.org/D19783
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19783: Fix cv-qualification of '*this' captures (and nasty bug PR27507 introduced by commit 263921 "Implement Lambda Capture of *this by Value as [=, *this]")
faisalv created this revision.
faisalv added reviewers: rsmith, hubert.reinterpretcast.
faisalv added subscribers: cfe-commits, gnzlbg.
The bug report by Gonzalo (https://llvm.org/bugs/show_bug.cgi?id=27507 -- which
results in clang crashing when generic lambdas that capture 'this' are
instantiated in contexts where the functionscopeinfo stack is not in a reliable
state - yet getCurrentThisType expects it to be) - unearthed some additional
bugs in regards to maintaining proper cv qualification through 'this' when
performing by value captures of '*this'.
This patch attempts to correct those bugs and makes the following changes:
- when capturing 'this', we do not need to remember the type of 'this' within
the LambdaScopeInfo's Capture - it is never really used for a this capture - so
remove it.
- teach getCurrentThisType to walk the stack of lambdas (even in scenarios
where we run out of LambdaScopeInfo's such as when instantiating call
operators) looking
for by copy captures of '*this' and resetting the type of 'this' based on
the constness of that capturing lambda's call operator.
As an example, consider the member function 'foo' below and follow along with
the static_asserts:
void foo() const {
auto L = [*this] () mutable {
static_assert(is_same);
++d;
auto M = [this] {
static_assert(is_same);
++d;
auto N = [] {
static_assert(is_same);
};
};
};
auto L1 = [*this] {
static_assert(is_same);
auto M = [this] () mutable {
static_assert(is_same);
auto N = [] {
static_assert(is_same);
};
};
};
auto L2 = [this] () mutable {
static_assert(is_same);
};
auto GL = [*this] (auto a) mutable {
static_assert(is_same);
++d;
auto M = [this] (auto b) {
static_assert(is_same);
++d;
auto N = [] (auto c) {
static_assert(is_same);
};
N(3.14);
};
M("abc");
};
GL(3.14);
}
Please see the test file for additional tests.
Thanks!
http://reviews.llvm.org/D19783
Files:
include/clang/Sema/ScopeInfo.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/cxx1z-lambda-star-this.cpp
Index: test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -3,6 +3,8 @@
// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS
// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING
+template constexpr bool is_same = false;
+template constexpr bool is_same = true;
namespace test_star_this {
namespace ns1 {
@@ -69,4 +71,112 @@
b.foo(); //expected-note{{in instantiation}}
} // end main
} // end ns4
+namespace ns5 {
+
+struct X {
+ double d = 3.14;
+ X(const volatile X&);
+ void foo() {
+
+ }
+
+ void foo() const { //expected-note{{const}}
+
+auto L = [*this] () mutable {
+ static_assert(is_same);
+ ++d;
+ auto M = [this] {
+static_assert(is_same);
+++d;
+auto N = [] {
+ static_assert(is_same);
+};
+ };
+};
+
+auto L1 = [*this] {
+ static_assert(is_same);
+ auto M = [this] () mutable {
+static_assert(is_same);
+auto N = [] {
+ static_assert(is_same);
+};
+ };
+};
+auto L2 = [this] () mutable {
+ static_assert(is_same);
+ ++d; //expected-error{{cannot assign}}
+};
+auto GL = [*this] (auto a) mutable {
+ static_assert(is_same);
+ ++d;
+ auto M = [this] (auto b) {
+static_assert(is_same);
+++d;
+auto N = [] (auto c) {
+ static_assert(is_same);
+};
+N(3.14);
+ };
+ M("abc");
+};
+GL(3.14);
+
+ }
+ void foo() volatile const {
+auto L = [this] () {
+ static_assert(is_same);
+ auto M = [*this] () mutable {
+static_assert(is_same);
+auto N = [this] {
+ static_assert(is_same);
+ auto M = [] {
+static_assert(is_same);
+ };
+};
+auto N2 = [*this] {
+ static_assert(is_same);
+};
+ };
+ auto M2 = [*this] () {
+static_assert(is_same);
+auto N = [this] {
+ static_assert(is_same);
+};
+ };
+};
+ }
+
+};
+
+} //end ns5
+namespace ns6 {
+struct X {
+ double d;
+ auto foo() const {
+auto L = [*this] () mutable {
+ auto M = [=] (auto a) {
+auto N = [this] {
+ ++d;
+ static_assert(is_same);
+ auto O = [*this] {
+static_assert(is_same);
+ };
+};
+N();
+static_assert(is_same);
+
Re: [PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h
vsk abandoned this revision. vsk added a comment. It is a bit messy. (Aside: I don't think the size of CodeGenPGO is visible with this change, so unique_ptr/default_delete wouldn't work?) http://reviews.llvm.org/D19243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h
Vedant Kumar writes:
> vsk created this revision.
> vsk added a reviewer: bogner.
> vsk added a subscriber: cfe-commits.
>
> Cons: 1 extra malloc per CodeGenFunction instantiation.
>
> Pros: This makes incremental builds noticeably faster, especially on
> my laptop. E.g with this patch there's no need to re-compile
> clang/lib/CodeGen/CGCUDARuntime.cpp InstrProfReader.h changes.
I doubt this is worthwhile. Mildly opposed.
If you do do this, use a unique_ptr rather than raw new/delete.
> http://reviews.llvm.org/D19243
>
> Files:
> lib/CodeGen/CGBlocks.cpp
> lib/CodeGen/CGCall.cpp
> lib/CodeGen/CGExpr.cpp
> lib/CodeGen/CGObjC.cpp
> lib/CodeGen/CGStmt.cpp
> lib/CodeGen/CGStmtOpenMP.cpp
> lib/CodeGen/CodeGenFunction.cpp
> lib/CodeGen/CodeGenFunction.h
> lib/CodeGen/CodeGenPGO.cpp
>
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -878,12 +878,33 @@
>
> llvm::MDNode *CodeGenFunction::createProfileWeightsForLoop(const Stmt *Cond,
> uint64_t
> LoopCount) {
> - if (!PGO.haveRegionCounts())
> + if (!PGO->haveRegionCounts())
> return nullptr;
> - Optional CondCount = PGO.getStmtCount(Cond);
> + Optional CondCount = PGO->getStmtCount(Cond);
>assert(CondCount.hasValue() && "missing expected loop condition count");
>if (*CondCount == 0)
> return nullptr;
>return createProfileWeights(LoopCount,
>std::max(*CondCount, LoopCount) - LoopCount);
> }
> +
> +void CodeGenFunction::incrementProfileCounter(const Stmt *S) {
> + if (CGM.getCodeGenOpts().hasProfileClangInstr())
> +PGO->emitCounterIncrement(Builder, S);
> + PGO->setCurrentStmt(S);
> +}
> +
> +uint64_t CodeGenFunction::getProfileCount(const Stmt *S) {
> + Optional Count = PGO->getStmtCount(S);
> + if (!Count.hasValue())
> +return 0;
> + return *Count;
> +}
> +
> +void CodeGenFunction::setCurrentProfileCount(uint64_t Count) {
> + PGO->setCurrentRegionCount(Count);
> +}
> +
> +uint64_t CodeGenFunction::getCurrentProfileCount() {
> + return PGO->getCurrentRegionCount();
> +}
> Index: lib/CodeGen/CodeGenFunction.h
> ===
> --- lib/CodeGen/CodeGenFunction.h
> +++ lib/CodeGen/CodeGenFunction.h
> @@ -19,7 +19,6 @@
> #include "CGLoopInfo.h"
> #include "CGValue.h"
> #include "CodeGenModule.h"
> -#include "CodeGenPGO.h"
> #include "EHScopeStack.h"
> #include "clang/AST/CharUnits.h"
> #include "clang/AST/ExprCXX.h"
> @@ -76,6 +75,7 @@
> class ObjCAutoreleasePoolStmt;
>
> namespace CodeGen {
> +class CodeGenPGO;
> class CodeGenTypes;
> class CGFunctionInfo;
> class CGRecordLayout;
> @@ -692,7 +692,7 @@
>/// block through the normal cleanup handling code (if any) and then
>/// on to \arg Dest.
>void EmitBranchThroughCleanup(JumpDest Dest);
> -
> +
>/// isObviouslyBranchWithoutCleanups - Return true if a branch to the
>/// specified destination obviously has no cleanups to run. 'false' is
> always
>/// a conservatively correct answer for this method.
> @@ -900,7 +900,7 @@
>if (Data.isValid()) Data.unbind(CGF);
> }
>};
> -
> +
> private:
>CGDebugInfo *DebugInfo;
>bool DisableDebugInfo;
> @@ -943,7 +943,7 @@
>};
>SmallVector BreakContinueStack;
>
> - CodeGenPGO PGO;
> + CodeGenPGO *PGO;
>
>/// Calculate branch weights appropriate for PGO data
>llvm::MDNode *createProfileWeights(uint64_t TrueCount, uint64_t
> FalseCount);
> @@ -953,30 +953,17 @@
>
> public:
>/// Increment the profiler's counter for the given statement.
> - void incrementProfileCounter(const Stmt *S) {
> -if (CGM.getCodeGenOpts().hasProfileClangInstr())
> - PGO.emitCounterIncrement(Builder, S);
> -PGO.setCurrentStmt(S);
> - }
> + void incrementProfileCounter(const Stmt *S);
>
>/// Get the profiler's count for the given statement.
> - uint64_t getProfileCount(const Stmt *S) {
> -Optional Count = PGO.getStmtCount(S);
> -if (!Count.hasValue())
> - return 0;
> -return *Count;
> - }
> + uint64_t getProfileCount(const Stmt *S);
>
>/// Set the profiler's current count.
> - void setCurrentProfileCount(uint64_t Count) {
> -PGO.setCurrentRegionCount(Count);
> - }
> + void setCurrentProfileCount(uint64_t Count);
>
>/// Get the profiler's current count. This is generally the count for the
> most
>/// recently incremented counter.
> - uint64_t getCurrentProfileCount() {
> -return PGO.getCurrentRegionCount();
> - }
> + uint64_t getCurrentProfileCount();
>
> private:
>
> Index: lib/CodeGen/CodeGenFunction.cpp
> ===
> --- lib/CodeGen/CodeGenFunction.cpp
> +++ lib/CodeGen/CodeGenFunction.cpp
> @@ -52,7 +52,7 @@
>ExceptionSlot(nullptr), EHSelectorS
Re: [PATCH] D19725: [Coverage] Fix an issue where a coverage region might not be created for a macro containing for or while statements.
Igor Kudrin writes:
> ikudrin created this revision.
> ikudrin added reviewers: bogner, davidxl, vsk.
> ikudrin added a subscriber: cfe-commits.
>
> The situation happened when a macro contained a full loop statement,
> which body ended at the end of the macro.
A comment and a nitpick, then LGTM.
> http://reviews.llvm.org/D19725
>
> Files:
> lib/CodeGen/CoverageMappingGen.cpp
> test/CoverageMapping/macroscopes.cpp
>
> Index: test/CoverageMapping/macroscopes.cpp
> ===
> --- test/CoverageMapping/macroscopes.cpp
> +++ test/CoverageMapping/macroscopes.cpp
> @@ -22,6 +22,17 @@
> #define starts_a_while while (x < 5)
> #define simple_stmt ++x
>
> +#define macro_with_for \
> + x = 3;\
> + for (int i = 0; i < x; ++i) { \
> + }
> +
> +#define macro_with_while \
> + x = 4; \
> + while (x < 5) {\
> +++x; \
> + }
> +
> // CHECK: main
> // CHECK-NEXT: File 0, [[@LINE+1]]:12 -> {{[0-9]+}}:2 = #0
> int main() {
> @@ -64,6 +75,11 @@
> simple_stmt;
>ends_a_scope
>
> + // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:17 = #0
> + macro_with_for
> + // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:19 = #0
> + macro_with_while
> +
>return 0;
> }
>
> @@ -103,3 +119,10 @@
> // CHECK-NEXT: File 11, 22:31 -> 22:36 = (#0 + #9)
> // CHECK-NEXT: File 12, 23:21 -> 23:24 = #9
> // CHECK-NEXT: File 13, 6:3 -> 7:4 = #9
> +// CHECK-NEXT: File 14, 26:3 -> 28:4 = #0
> +// CHECK-NEXT: File 14, 27:19 -> 27:24 = (#0 + #10)
> +// CHECK-NEXT: File 14, 27:26 -> 27:29 = #10
> +// CHECK-NEXT: File 14, 27:31 -> 28:4 = #10
> +// CHECK-NEXT: File 15, 31:3 -> 34:4 = #0
> +// CHECK-NEXT: File 15, 32:10 -> 32:15 = (#0 + #11)
> +// CHECK-NEXT: File 15, 32:17 -> 34:4 = #11
> Index: lib/CodeGen/CoverageMappingGen.cpp
> ===
> --- lib/CodeGen/CoverageMappingGen.cpp
> +++ lib/CodeGen/CoverageMappingGen.cpp
> @@ -443,15 +443,30 @@
> return ExitCount;
>}
>
> + /// \brief Check whether a region with bounds \c StartLoc and \c EndLoc
> + /// is already added to \c SourceRegions.
> + bool isRegionAlreadyAdded(SourceLocation StartLoc, SourceLocation EndLoc) {
> +return SourceRegions.rend() !=
> + std::find_if(SourceRegions.rbegin(), SourceRegions.rend(),
Any particular reason for rbegin/rend? I guess you expect the match to
be near the end of the list when it exists?
> +[=](const SourceMappingRegion &Region) {
There's no good reason to capture by value here. Best to use [&] for
efficiency.
> + assert(Region.hasStartLoc() && "incomplete
> region");
> + assert(Region.hasEndLoc() && "incomplete region");
> + return Region.getStartLoc() == StartLoc &&
> + Region.getEndLoc() == EndLoc;
> +});
> + }
> +
>/// \brief Adjust the most recently visited location to \c EndLoc.
>///
>/// This should be used after visiting any statements in non-source order.
>void adjustForOutOfOrderTraversal(SourceLocation EndLoc) {
> MostRecentLocation = EndLoc;
> // Avoid adding duplicate regions if we have a completed region on the
> top
> -// of the stack and are adjusting to the end of a virtual file.
> +// of the stack.
> if (getRegion().hasEndLoc() &&
> -MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation))
> +MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation) &&
> +isRegionAlreadyAdded(getStartOfFileOrMacro(MostRecentLocation),
> + MostRecentLocation))
>MostRecentLocation = getIncludeOrExpansionLoc(MostRecentLocation);
>}
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18495: Check default arguments of template template parameters for compatibility.
faisalv added a comment. *ping* http://reviews.llvm.org/D18495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18510: [cxx1z-constexpr-lambda] Make conversion function constexpr
faisalv added a comment. *ping* http://reviews.llvm.org/D18510 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.
etienneb added a comment.
this is cool :)
It's a simple and precise rule to avoid using the probabilistic heuristic.
Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:94
@@ -93,3 +93,3 @@
Finder->addMatcher(StringsInitializerList.bind("list"), this);
}
If it's working as-is,... this is neat :)
Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:106
@@ +105,3 @@
+ if (InitializerList->hasArrayFiller()) {
+ diag(InitializerList->getExprLoc(),
+ "wrong string array initialization: "
The error should still be reported to the missing comma (concatenated token):
ConcatenatedLiteral->getLocStart(),
We could add a NOTE to point to the array, stating that the size mismatch.
What do you think?
Comment at: test/clang-tidy/misc-suspicious-missing-comma.cpp:84
@@ +83,3 @@
+
+// Missing comma or wrong explicit array size.
+const char* TheThreeMusketeers[4] = {
Could you add the more complicated example I sent you.
More tests is always welcome.
Currently, there is no tests for initialisation list in initialisation list.
http://reviews.llvm.org/D19769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC
faisalv added a comment. Thanks for your response. I don't feel strongly about either of those issues - especially since it seems you considered the options - and have good reasons for choosing the alternative. I would also favor adding in a comment an example of how users are expected to use this functionality to create such objects on the stack. Thanks! P.S. Since you weren't seeing my previous comments emailed to you from phabricator - could you let me know if this one is? http://reviews.llvm.org/D19770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19385: [scan-build] fix logic error warnings emitted on clang code base
apelete updated this revision to Diff 55757.
apelete added a comment.
[scan-build] fix logic error warnings emitted on clang code base
Changes since last revision:
- lib/Format/AffectedRangeManager.cpp: fix typo in class name.
http://reviews.llvm.org/D19385
Files:
lib/AST/DeclObjC.cpp
lib/Format/AffectedRangeManager.cpp
lib/Format/AffectedRangeManager.h
lib/Frontend/CompilerInstance.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -297,6 +297,7 @@
if (!Diags.empty())
SM = &Diags.front()->path.front()->getLocation().getManager();
+ assert(SM && "SourceManager is NULL, cannot iterate through the diagnostics");
for (std::vector::iterator DI = Diags.begin(),
DE = Diags.end(); DI != DE; ++DI) {
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3744,6 +3744,7 @@
bool AnyVisibleDecls = !NewDecls.empty();
for (/**/; DI != DE; ++DI) {
+assert(*DI && "TypoCorrection iterator cannot be NULL");
NamedDecl *VisibleDecl = *DI;
if (!LookupResult::isVisible(SemaRef, *DI))
VisibleDecl = findAcceptableDecl(SemaRef, *DI);
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -398,8 +398,10 @@
SourceLocation TypeidLoc,
Expr *E,
SourceLocation RParenLoc) {
+ assert(E && "expression operand is NULL, cannot build C++ typeid expression");
+
bool WasEvaluated = false;
- if (E && !E->isTypeDependent()) {
+ if (!E->isTypeDependent()) {
if (E->getType()->isPlaceholderType()) {
ExprResult result = CheckPlaceholderExpr(E);
if (result.isInvalid()) return ExprError();
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -742,7 +742,7 @@
// Figure out where to get and map in the main file.
if (InputFile != "-") {
-const FileEntry *File;
+const FileEntry *File = nullptr;
if (Opts.FindPchSource.empty()) {
File = FileMgr.getFile(InputFile, /*OpenFile=*/true);
} else {
@@ -760,13 +760,14 @@
SmallVector, 16>
Includers;
Includers.push_back(std::make_pair(FindFile, FindFile->getDir()));
- File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
-/*FromDir=*/nullptr,
-/*CurDir=*/UnusedCurDir, Includers,
-/*SearchPath=*/nullptr,
-/*RelativePath=*/nullptr,
-/*RequestingModule=*/nullptr,
-/*SuggestedModule=*/nullptr, /*SkipCache=*/true);
+ if (HS)
+File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
+ /*FromDir=*/nullptr,
+ /*CurDir=*/UnusedCurDir, Includers,
+ /*SearchPath=*/nullptr,
+ /*RelativePath=*/nullptr,
+ /*RequestingModule=*/nullptr,
+ /*SuggestedModule=*/nullptr, /*SkipCache=*/true);
// Also add the header to /showIncludes output.
if (File)
DepOpts.ShowIncludesPretendHeader = File->getName();
Index: lib/Format/AffectedRangeManager.h
===
--- lib/Format/AffectedRangeManager.h
+++ lib/Format/AffectedRangeManager.h
@@ -54,7 +54,7 @@
// Determines whether 'Line' is affected by the SourceRanges given as input.
// Returns \c true if line or one if its children is affected.
- bool nonPPLineAffected(AnnotatedLine *Line,
+ bool nonPPLineAffected(AnnotatedLine &Line,
const AnnotatedLine *PreviousLine);
const SourceManager &SourceMgr;
Index: lib/Format/AffectedRangeManager.cpp
===
--- lib/Format/AffectedRangeManager.cpp
+++ lib/Format/AffectedRangeManager.cpp
@@ -48,7 +48,7 @@
continue;
}
-if (nonPPLineAffected(Line, PreviousLine))
+if (nonPPLineAffected(*Line, PreviousLine))
SomeLineAffected = true;
PreviousLine = Line;
@@ -99,24 +99,27 @@
}
bool AffectedRangeManager::nonPPLineAffected(
-AnnotatedLine *Line, const AnnotatedLine *PreviousLine) {
+AnnotatedLine &Line, const AnnotatedLine *PreviousLine) {
bool SomeLineAffected = false;
- Line->ChildrenAffected =
-
Re: r261717 - Default vaarg lowering should support indirect struct types.
On Wed, Feb 24, 2016 at 08:38:24AM -0800, Hans Wennborg via cfe-commits wrote: > On Wed, Feb 24, 2016 at 5:11 AM, Joerg Sonnenberger via cfe-commits > wrote: > > On Wed, Feb 24, 2016 at 03:03:32AM -, James Y Knight via cfe-commits > > wrote: > >> Author: jyknight > >> Date: Tue Feb 23 20:59:33 2016 > >> New Revision: 261717 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=261717&view=rev > >> Log: > >> Default vaarg lowering should support indirect struct types. > >> > >> Fixes PR11517 for SPARC. > > > > It would be nice to get the PPC32 and XCore parts finished by those > > involved with the platforms, but I would like to see this merged into > > the 3.8 branch at some point after 3.8.0. It is one of the missing basic > > codegen items for SPARC. > > I've put it on the list. Tom? Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r262838 - Implement __builtin_eh_return_data_regno for SPARC and SPARC64.
On Mon, Mar 07, 2016 at 06:30:52PM +0100, Joerg Sonnenberger via cfe-commits wrote: > On Mon, Mar 07, 2016 at 05:19:16PM -, Joerg Sonnenberger via cfe-commits > wrote: > > Author: joerg > > Date: Mon Mar 7 11:19:15 2016 > > New Revision: 262838 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=262838&view=rev > > Log: > > Implement __builtin_eh_return_data_regno for SPARC and SPARC64. > > Hans, please merge this into the 3.8 branch, it unbreaks personality > routines on SPARC/SPARC64. Tom? Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC
hubert.reinterpretcast planned changes to this revision.
hubert.reinterpretcast added a comment.
The alias template hit a build-compiler bug: the substitution of the pack
expansion of `Counts` does not work properly. I am investigating a replacement
with a templated struct with a typedef member.
Comment at: include/llvm/Support/TrailingObjects.h:352
@@ +351,3 @@
+template
+using _ =
+llvm::AlignedCharArray::Alignment,
faisalv wrote:
> FWIW - while I see your point - the 'underscore' as a name seems out-of-style
> - as compared to what i'm used to reading within the llvm/clang code-base.
> Personally I wouldn't mind calling it 'Type' or 'Ty'.
>
> Also what are your thoughts about instead of making it a struct (and
> providing a more generic solution), just making it a template alias,
> constrained on the enclosing TrailingTypes pack (i.e. specialized for only
> being able to provide an answer for the enclosing type)?
> For e.g - something along the lines of
> template using FixedSizeStorageType =
> ...totalSizeToAlloc(Counts...)>;
>
>
>
It only provides an answer for the enclosing type: `totalSizeToAlloc` is
constrained.
The reason for the extra layer is to replicate the redundant specification that
`totalSizeToAlloc` has.
Comment at: include/llvm/Support/TrailingObjects.h:359
@@ +358,3 @@
+ class FixedSizeStorageOwner {
+ public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
faisalv wrote:
> What are your thoughts on having this guy be templated on
> and actually contain the Storage as a data member itself (as opposed to
> requiring folks to declare it - as you do for your Fixed TPL patch), and
> hiding the placement new under a forwarding templated constructor that
> forwards to BaseTy's ctors?
>
> I imagine folks could just use it then as [If BaseTy = TemplateParameterList]:
> TemplateParameterList::FixedSizeTrailingCounts<5> TPL(ctors-arguments);
>
>
>
I much prefer not to introduce forwarding templated constructors (at least
without introducing a tag-wart). The smart-pointer idiom is well understood,
and I would not want to step into novel design territory in production code.
http://reviews.llvm.org/D19770
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19385: [scan-build] fix logic error warnings emitted on clang code base
apelete added a comment. Waiting for review, could someone please have a look at this one ? http://reviews.llvm.org/D19385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19278: [scan-build] fix logic error warnings emitted on clang code base
apelete added a reviewer: pcc. apelete added a comment. Waiting for review, could someone please have a look at this one ? http://reviews.llvm.org/D19278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19084: [clang-analyzer] fix warnings emitted on clang code base
apelete added a reviewer: rjmccall. apelete added a comment. Waiting for review, could someone please have a look at this one ? http://reviews.llvm.org/D19084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19385: [scan-build] fix logic error warnings emitted on clang code base
apelete updated this revision to Diff 55748.
apelete added a comment.
[scan-build] fix logic error warnings emitted on clang code base
Changes since last revision:
- fast forward rebase on git master branch.
http://reviews.llvm.org/D19385
Files:
lib/AST/DeclObjC.cpp
lib/Format/AffectedRangeManager.cpp
lib/Format/AffectedRangeManager.h
lib/Frontend/CompilerInstance.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -297,6 +297,7 @@
if (!Diags.empty())
SM = &Diags.front()->path.front()->getLocation().getManager();
+ assert(SM && "SourceManager is NULL, cannot iterate through the diagnostics");
for (std::vector::iterator DI = Diags.begin(),
DE = Diags.end(); DI != DE; ++DI) {
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3744,6 +3744,7 @@
bool AnyVisibleDecls = !NewDecls.empty();
for (/**/; DI != DE; ++DI) {
+assert(*DI && "TypoCorrection iterator cannot be NULL");
NamedDecl *VisibleDecl = *DI;
if (!LookupResult::isVisible(SemaRef, *DI))
VisibleDecl = findAcceptableDecl(SemaRef, *DI);
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -398,8 +398,10 @@
SourceLocation TypeidLoc,
Expr *E,
SourceLocation RParenLoc) {
+ assert(E && "expression operand is NULL, cannot build C++ typeid expression");
+
bool WasEvaluated = false;
- if (E && !E->isTypeDependent()) {
+ if (!E->isTypeDependent()) {
if (E->getType()->isPlaceholderType()) {
ExprResult result = CheckPlaceholderExpr(E);
if (result.isInvalid()) return ExprError();
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -742,7 +742,7 @@
// Figure out where to get and map in the main file.
if (InputFile != "-") {
-const FileEntry *File;
+const FileEntry *File = nullptr;
if (Opts.FindPchSource.empty()) {
File = FileMgr.getFile(InputFile, /*OpenFile=*/true);
} else {
@@ -760,13 +760,14 @@
SmallVector, 16>
Includers;
Includers.push_back(std::make_pair(FindFile, FindFile->getDir()));
- File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
-/*FromDir=*/nullptr,
-/*CurDir=*/UnusedCurDir, Includers,
-/*SearchPath=*/nullptr,
-/*RelativePath=*/nullptr,
-/*RequestingModule=*/nullptr,
-/*SuggestedModule=*/nullptr, /*SkipCache=*/true);
+ if (HS)
+File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
+ /*FromDir=*/nullptr,
+ /*CurDir=*/UnusedCurDir, Includers,
+ /*SearchPath=*/nullptr,
+ /*RelativePath=*/nullptr,
+ /*RequestingModule=*/nullptr,
+ /*SuggestedModule=*/nullptr, /*SkipCache=*/true);
// Also add the header to /showIncludes output.
if (File)
DepOpts.ShowIncludesPretendHeader = File->getName();
Index: lib/Format/AffectedRangeManager.h
===
--- lib/Format/AffectedRangeManager.h
+++ lib/Format/AffectedRangeManager.h
@@ -54,7 +54,7 @@
// Determines whether 'Line' is affected by the SourceRanges given as input.
// Returns \c true if line or one if its children is affected.
- bool nonPPLineAffected(AnnotatedLine *Line,
+ bool nonPPLineAffected(AnnotatedLine &Line,
const AnnotatedLine *PreviousLine);
const SourceManager &SourceMgr;
Index: lib/Format/AffectedRangeManager.cpp
===
--- lib/Format/AffectedRangeManager.cpp
+++ lib/Format/AffectedRangeManager.cpp
@@ -48,7 +48,7 @@
continue;
}
-if (nonPPLineAffected(Line, PreviousLine))
+if (nonPPLineAffected(*Line, PreviousLine))
SomeLineAffected = true;
PreviousLine = Line;
@@ -98,25 +98,28 @@
}
}
-bool AffectedRangeManager::nonPPLineAffected(
-AnnotatedLine *Line, const AnnotatedLine *PreviousLine) {
+bool AffectedRangeMLanager::nonPPLineAffected(
+AnnotatedLine &Line, const AnnotatedLine *PreviousLine) {
bool SomeLineAffected = false;
Re: [PATCH] D19278: [scan-build] fix logic error warnings emitted on clang code base
apelete updated this revision to Diff 55747.
apelete added a comment.
[scan-build] fix logic error warnings emitted on clang code base
Changes since last revision:
- fast forward rebase on git master branch.
http://reviews.llvm.org/D19278
Files:
lib/AST/ExprConstant.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenModule.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaOpenMP.cpp
lib/Sema/SemaOverload.cpp
lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -693,6 +693,8 @@
!Param->getType()->isReferenceType())
continue;
+assert(ArgExpr && "cannot get the type of a NULL expression");
+
NullConstraint Nullness = getNullConstraint(*ArgSVal, State);
Nullability RequiredNullability =
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -8705,6 +8705,7 @@
void Sema::diagnoseEquivalentInternalLinkageDeclarations(
SourceLocation Loc, const NamedDecl *D, ArrayRef Equiv) {
+ assert(D && "named declaration must be not NULL");
Diag(Loc, diag::ext_equivalent_internal_linkage_decl_in_modules) << D;
Module *M = getOwningModule(const_cast(D));
@@ -9185,7 +9186,9 @@
!ToRefTy->getPointeeType()->isIncompleteType() &&
S.IsDerivedFrom(SourceLocation(), ToRefTy->getPointeeType(), FromTy)) {
BaseToDerivedConversion = 3;
-} else if (ToTy->isLValueReferenceType() && !FromExpr->isLValue() &&
+} else if (FromExpr &&
+ !FromExpr->isLValue() &&
+ ToTy->isLValueReferenceType() &&
ToTy.getNonReferenceType().getCanonicalType() ==
FromTy.getNonReferenceType().getCanonicalType()) {
S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_lvalue)
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -985,6 +985,7 @@
(VD && DSAStack->isForceVarCapturing()))
return VD ? VD : Info.second;
auto DVarPrivate = DSAStack->getTopDSA(D, DSAStack->isClauseParsingMode());
+assert(DVarPrivate.PrivateCopy && "DSAStackTy object must be not NULL");
if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind))
return VD ? VD : cast(DVarPrivate.PrivateCopy->getDecl());
DVarPrivate = DSAStack->hasDSA(D, isOpenMPPrivate, MatchesAlways(),
@@ -3994,6 +3995,7 @@
static ExprResult
tryBuildCapture(Sema &SemaRef, Expr *Capture,
llvm::MapVector &Captures) {
+ assert(Capture && "cannot build capture if expression is NULL");
if (Capture->isEvaluatable(SemaRef.Context, Expr::SE_AllowSideEffects))
return SemaRef.PerformImplicitConversion(
Capture->IgnoreImpCasts(), Capture->getType(), Sema::AA_Converting,
@@ -4251,7 +4253,7 @@
SemaRef.Diag(CollapseLoopCountExpr->getExprLoc(),
diag::note_omp_collapse_ordered_expr)
<< 0 << CollapseLoopCountExpr->getSourceRange();
- else
+ else if (OrderedLoopCountExpr)
SemaRef.Diag(OrderedLoopCountExpr->getExprLoc(),
diag::note_omp_collapse_ordered_expr)
<< 1 << OrderedLoopCountExpr->getSourceRange();
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -434,6 +434,9 @@
/// error, false otherwise.
bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
Scope *S) {
+ assert(New && "New function declaration is NULL, aborting merge.");
+ assert(Old && "Old function declaration is NULL, aborting merge.");
+
bool Invalid = false;
// The declaration context corresponding to the scope is the semantic
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2299,6 +2299,7 @@
unsigned CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D,
unsigned AddrSpace) {
+ assert(D && "variable declaration must be not NULL");
if (LangOpts.CUDA && LangOpts.CUDAIsDevice) {
if (D->hasAttr())
AddrSpace = getContext().getTargetAddressSpace(LangAS::cuda_constant);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1314,6 +1314,7 @@
CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
}
+ assert(V && "consta
Re: [PATCH] D19084: [clang-analyzer] fix warnings emitted on clang code base
apelete updated this revision to Diff 55746.
apelete added a comment.
[clang-analyzer] fix warnings emitted on clang code base
Changes since last revision:
- fast forward rebase on git master branch.
http://reviews.llvm.org/D19084
Files:
lib/AST/ASTDiagnostic.cpp
lib/AST/NestedNameSpecifier.cpp
lib/Driver/Tools.cpp
lib/Sema/SemaInit.cpp
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4862,6 +4862,8 @@
InitializationSequence &Sequence,
const InitializedEntity &Entity,
Expr *Initializer) {
+ assert(Initializer && "Initializer needs to be not NULL");
+
bool ArrayDecay = false;
QualType ArgType = Initializer->getType();
QualType ArgPointee;
@@ -5237,11 +5239,11 @@
DeclAccessPair dap;
if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer)) {
AddZeroInitializationStep(Entity.getType());
-} else if (Initializer->getType() == Context.OverloadTy &&
+} else if (Initializer && Initializer->getType() == Context.OverloadTy &&
!S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
false, dap))
SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
-else if (Initializer->getType()->isFunctionType() &&
+else if (Initializer && Initializer->getType()->isFunctionType() &&
isExprAnUnaddressableFunction(S, Initializer))
SetFailed(InitializationSequence::FK_AddressOfUnaddressableFunction);
else
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2346,7 +2346,7 @@
success = getAArch64MicroArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args),
Args, Features);
- if (!success)
+ if (!success && A)
D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/AST/NestedNameSpecifier.cpp
+++ lib/AST/NestedNameSpecifier.cpp
@@ -456,7 +456,9 @@
Buffer = NewBuffer;
BufferCapacity = NewCapacity;
}
-
+
+assert(Buffer && "Buffer cannot be NULL");
+
memcpy(Buffer + BufferSize, Start, End - Start);
BufferSize += End-Start;
}
Index: lib/AST/ASTDiagnostic.cpp
===
--- lib/AST/ASTDiagnostic.cpp
+++ lib/AST/ASTDiagnostic.cpp
@@ -1683,7 +1683,7 @@
ToName = ToTD->getQualifiedNameAsString();
}
-if (Same) {
+if (Same && FromTD) {
OS << "template " << FromTD->getNameAsString();
} else if (!PrintTree) {
OS << (FromDefault ? "(default) template " : "template ");
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4862,6 +4862,8 @@
InitializationSequence &Sequence,
const InitializedEntity &Entity,
Expr *Initializer) {
+ assert(Initializer && "Initializer needs to be not NULL");
+
bool ArrayDecay = false;
QualType ArgType = Initializer->getType();
QualType ArgPointee;
@@ -5237,11 +5239,11 @@
DeclAccessPair dap;
if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer)) {
AddZeroInitializationStep(Entity.getType());
-} else if (Initializer->getType() == Context.OverloadTy &&
+} else if (Initializer && Initializer->getType() == Context.OverloadTy &&
!S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
false, dap))
SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
-else if (Initializer->getType()->isFunctionType() &&
+else if (Initializer && Initializer->getType()->isFunctionType() &&
isExprAnUnaddressableFunction(S, Initializer))
SetFailed(InitializationSequence::FK_AddressOfUnaddressableFunction);
else
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2346,7 +2346,7 @@
success = getAArch64MicroArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args),
Args, Features);
- if (!success)
+ if (!success && A)
D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/
Re: [PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC
faisalv added a comment.
Thanks for running with this! =)
Comment at: include/llvm/Support/TrailingObjects.h:352
@@ +351,3 @@
+template
+using _ =
+llvm::AlignedCharArray::Alignment,
FWIW - while I see your point - the 'underscore' as a name seems out-of-style -
as compared to what i'm used to reading within the llvm/clang code-base.
Personally I wouldn't mind calling it 'Type' or 'Ty'.
Also what are your thoughts about instead of making it a struct (and providing
a more generic solution), just making it a template alias, constrained on the
enclosing TrailingTypes pack (i.e. specialized for only being able to provide
an answer for the enclosing type)?
For e.g - something along the lines of
template using FixedSizeStorageType =
...totalSizeToAlloc(Counts...)>;
Comment at: include/llvm/Support/TrailingObjects.h:359
@@ +358,3 @@
+ class FixedSizeStorageOwner {
+ public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
What are your thoughts on having this guy be templated on
and actually contain the Storage as a data member itself (as opposed to
requiring folks to declare it - as you do for your Fixed TPL patch), and hiding
the placement new under a forwarding templated constructor that forwards to
BaseTy's ctors?
I imagine folks could just use it then as [If BaseTy = TemplateParameterList]:
TemplateParameterList::FixedSizeTrailingCounts<5> TPL(ctors-arguments);
http://reviews.llvm.org/D19770
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r268196 - [clang][Builtin][AVX512] Adding intrinsics for vmovshdup and vmovsldup instruction set
Author: mzuckerm
Date: Sun May 1 09:43:43 2016
New Revision: 268196
URL: http://llvm.org/viewvc/llvm-project?rev=268196&view=rev
Log:
[clang][Builtin][AVX512] Adding intrinsics for vmovshdup and vmovsldup
instruction set
Differential Revision: http://reviews.llvm.org/D19595
Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c
Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=268196&r1=268195&r2=268196&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sun May 1 09:43:43 2016
@@ -2224,6 +2224,12 @@ TARGET_BUILTIN(__builtin_ia32_compresssf
TARGET_BUILTIN(__builtin_ia32_compresssi512_mask,
"V16iV16iV16iUs","","avx512f")
TARGET_BUILTIN(__builtin_ia32_cmpsd_mask, "UcV2dV2dIiUcIi","","avx512f")
TARGET_BUILTIN(__builtin_ia32_cmpss_mask, "UcV4fV4fIiUcIi","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_movshdup512_mask, "V16fV16fV16fUs","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_movsldup512_mask, "V16fV16fV16fUs","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_movshdup128_mask, "V4fV4fV4fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_movshdup256_mask, "V8fV8fV8fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_movsldup128_mask, "V4fV4fV4fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_movsldup256_mask, "V8fV8fV8fUc","","avx512vl")
#undef BUILTIN
#undef TARGET_BUILTIN
Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=268196&r1=268195&r2=268196&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Sun May 1 09:43:43 2016
@@ -7681,6 +7681,58 @@ __builtin_ia32_cmpsd_mask ((__v2df)( __X
_MM_FROUND_CUR_DIRECTION);\
})
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_movehdup_ps (__m512 __A)
+{
+ return (__m512) __builtin_ia32_movshdup512_mask ((__v16sf) __A,
+ (__v16sf)
+ _mm512_undefined_ps (),
+ (__mmask16) -1);
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_mask_movehdup_ps (__m512 __W, __mmask16 __U, __m512 __A)
+{
+ return (__m512) __builtin_ia32_movshdup512_mask ((__v16sf) __A,
+ (__v16sf) __W,
+ (__mmask16) __U);
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_maskz_movehdup_ps (__mmask16 __U, __m512 __A)
+{
+ return (__m512) __builtin_ia32_movshdup512_mask ((__v16sf) __A,
+ (__v16sf)
+ _mm512_setzero_ps (),
+ (__mmask16) __U);
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_moveldup_ps (__m512 __A)
+{
+ return (__m512) __builtin_ia32_movsldup512_mask ((__v16sf) __A,
+ (__v16sf)
+ _mm512_undefined_ps (),
+ (__mmask16) -1);
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_mask_moveldup_ps (__m512 __W, __mmask16 __U, __m512 __A)
+{
+ return (__m512) __builtin_ia32_movsldup512_mask ((__v16sf) __A,
+ (__v16sf) __W,
+ (__mmask16) __U);
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_maskz_moveldup_ps (__mmask16 __U, __m512 __A)
+{
+ return (__m512) __builtin_ia32_movsldup512_mask ((__v16sf) __A,
+ (__v16sf)
+ _mm512_setzero_ps (),
+ (__mmask16) __U);
+}
+
#undef __DEFAULT_FN_ATTRS
#endif // __AVX512FINTRIN_H
Modified: cfe/trunk/lib/Headers/avx512vlintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vlintrin.h?rev=268196&r1=268195&r2=268196&view=diff
==
--- cfe/trunk/lib/Headers/avx512vlintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vlintrin.h Sun May 1 09:43:43 2016
@@ -9293,6 +9293,74 @@ __builtin_ia32_alignq256_mask ((__v4di)(
(__mmask8)( __U));\
})
+static __inline__ __m128 __DEFAULT_FN_ATTRS
+_mm_mask_movehdup_ps (__m128 __W, __mmask8 __U, __m128 __A)
+{
+ return (__m128) __builtin_ia32_movshdup128_mask ((__v4sf) __A,
+ (__v4sf) __W,
+ (__mmask8) __U);
+}
+
+static __inline__ __m128 __DEFAULT_FN_ATTRS
+_mm_maskz_movehdup_ps (__mmask8 __U, __m128 __A)
+{
+ return (__m128) __builtin_ia32_movshdup128_mask ((__v4sf) __A,
+ (__v4sf)
+ _mm_setzero_ps (),
+ (__mmask8) __U);
+}
+
+static __inline__ __m256 __DEFAULT_FN_ATTRS
+_mm256_mask_movehdup_ps (__m256 __W, __mmask8 __U, __m256 __A)
+{
+ return (__m256) __builtin_ia32_movshdup256_mask ((__v8sf) __A,
+ (__
Re: [PATCH] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.
szdominik updated this revision to Diff 55741.
szdominik added a comment.
Implement the heuristic in a different (and simpler) way.
Based on the array fillers.
http://reviews.llvm.org/D19769
Files:
clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
test/clang-tidy/misc-suspicious-missing-comma.cpp
Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -80,3 +80,21 @@
"Dummy line",
"Dummy line",
};
+
+// Missing comma or wrong explicit array size.
+const char* TheThreeMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis"
+ "D'Artagnan"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: wrong string array initialization:
the explicit given size and the real number of elements aren't equal
[misc-suspicious-missing-comma]
+
+// Correctly given array size should avoid warning.
+const char* TheFourMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis",
+ "Charles de Batz de Castelmore"
+ "D'Artagnan"
+};
Index: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
===
--- docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
+++ docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
@@ -42,3 +42,14 @@
"Warning %s",
};
+This checker is also capable of warn on cases when the explicitly given array
size
+isn't equal to the real array size.
+
+.. code:: c++
+
+ const char* TheThreeMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis"
+ "D'Artagnan"
+};
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
@@ -100,6 +100,16 @@
Result.Nodes.getNodeAs("str");
assert(InitializerList && ConcatenatedLiteral);
+ // InitializerList has an array filler when the explicitly given size is
+ // bigger than the real array size - e.g. because there is a missing comma.
+ if (InitializerList->hasArrayFiller()) {
+ diag(InitializerList->getExprLoc(),
+ "wrong string array initialization: "
+ "the explicit given size and the "
+ "real number of elements aren't equal");
+ return;
+ }
+
// Skip small arrays as they often generate false-positive.
unsigned int Size = InitializerList->getNumInits();
if (Size < SizeThreshold) return;
Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -80,3 +80,21 @@
"Dummy line",
"Dummy line",
};
+
+// Missing comma or wrong explicit array size.
+const char* TheThreeMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis"
+ "D'Artagnan"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: wrong string array initialization: the explicit given size and the real number of elements aren't equal [misc-suspicious-missing-comma]
+
+// Correctly given array size should avoid warning.
+const char* TheFourMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis",
+ "Charles de Batz de Castelmore"
+ "D'Artagnan"
+};
Index: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
===
--- docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
+++ docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
@@ -42,3 +42,14 @@
"Warning %s",
};
+This checker is also capable of warn on cases when the explicitly given array size
+isn't equal to the real array size.
+
+.. code:: c++
+
+ const char* TheThreeMusketeers[4] = {
+ "Athos",
+ "Porthos",
+ "Aramis"
+ "D'Artagnan"
+};
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
@@ -100,6 +100,16 @@
Result.Nodes.getNodeAs("str");
assert(InitializerList && ConcatenatedLiteral);
+ // InitializerList has an array filler when the explicitly given size is
+ // bigger than the real array size - e.g. because there is a missing comma.
+ if (InitializerList->hasArrayFiller()) {
+ diag(InitializerList->getExprLoc(),
+ "wrong string array initialization: "
+ "the explicit given size and the "
+ "real number of elements aren't equal");
+ return;
+ }
+
// Skip small arrays as they often generate false-positive.
unsigned int Size = InitializerList->getNumInits();
if (Size < SizeThreshold) return;
___
cfe-commits mailing list
cfe-commi
Re: [PATCH] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.
szdominik added inline comments.
Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:92
@@ +91,3 @@
+ has(expr(ignoringImpCasts(ConcatenatedStringLiteral))),
+ hasParent(varDecl(allOf(hasType(arrayType()), isDefinition()))
+.bind("varDecl")));
etienneb wrote:
> The issue I see by matching the "hasParent" that way is that you are assuming
> that the matcher is only looking for initialisation-list attached to variable
> declaration (direct child).
>
> Assume this case:
> ```
> struct StrArray {
> int n;
> const char* list[5];
> } A[2] = {
> {5, {"a", "b", "c", "d", "e" }},
> {5, {"a", "b", "c", "d", "e" }},
> };
> ```
>
> It is giving you this AST representation:
> ```
> VarDecl 0x1e82738 line:9:1> line:6:3 A 'struct StrArray [2]' cinit
> `-InitListExpr 0x1e82c38 'struct StrArray [2]'
> |-InitListExpr 0x1e82c88 'struct StrArray':'struct
> StrArray'
> | |-IntegerLiteral 0x1e827d8 'int' 5
> | `-InitListExpr 0x1e82cd8 'const char *[5]'
> | |-ImplicitCastExpr 0x1e82d40 'const char *'
>
> | | `-StringLiteral 0x1e82878 'const char [2]' lvalue "a"
> | |-ImplicitCastExpr 0x1e82d58 'const char *'
>
> | | `-StringLiteral 0x1e828a8 'const char [2]' lvalue "b"
> | |-ImplicitCastExpr 0x1e82d70 'const char *'
>
> | | `-StringLiteral 0x1e828d8 'const char [2]' lvalue "c"
> | |-ImplicitCastExpr 0x1e82d88 'const char *'
>
> | | `-StringLiteral 0x1e82908 'const char [2]' lvalue "d"
> | `-ImplicitCastExpr 0x1e82da0 'const char *'
>
> | `-StringLiteral 0x1e82938 'const char [2]' lvalue "e"
> `-InitListExpr 0x1e82db8 'struct StrArray':'struct
> StrArray'
> |-IntegerLiteral 0x1e82a20 'int' 5
> `-InitListExpr 0x1e82e08 'const char *[5]'
> |-ImplicitCastExpr 0x1e82e70 'const char *'
>
> | `-StringLiteral 0x1e82a40 'const char [2]' lvalue "a"
> |-ImplicitCastExpr 0x1e82e88 'const char *'
>
> | `-StringLiteral 0x1e82a70 'const char [2]' lvalue "b"
> |-ImplicitCastExpr 0x1e82ea0 'const char *'
>
> | `-StringLiteral 0x1e82aa0 'const char [2]' lvalue "c"
> |-ImplicitCastExpr 0x1e82eb8 'const char *'
>
> | `-StringLiteral 0x1e82ad0 'const char [2]' lvalue "d"
> `-ImplicitCastExpr 0x1e82ed0 'const char *'
>
> `-StringLiteral 0x1e82b00 'const char [2]' lvalue "e"
> ```
>
> I believe this can be solved with something like:
> hasParent(anyOf(varDecl(allOf(hasType(arrayType()), isDefinition()),
> anything())) <<-- to accept all other
> cases.
>
> That way, you are adding an heuristic to filter some incorrect warnings.
>
> I believe it's possible to match the example above as the size is part of the
> type.
>
> If I try this example:
>
> ```
> {5, {"a", "b", "c", "d"}},// only four string literal
> ```
>
> I'm getting the following AST:
> ```
> `-InitListExpr 0x28ffd50 'struct StrArray':'struct
> StrArray'
> |-IntegerLiteral 0x28ff9e8 'int' 5
> `-InitListExpr 0x28ffda0 'const char *[5]'
> |-array filler
> | `-ImplicitValueInitExpr 0x28ffe78 <> 'const char *'
> |-ImplicitCastExpr 0x28ffde0 'const char *'
>
> | `-StringLiteral 0x28ffa08 'const char [2]' lvalue "a"
> |-ImplicitCastExpr 0x28ffe00 'const char *'
>
> | `-StringLiteral 0x28ffa38 'const char [2]' lvalue "b"
> |-ImplicitCastExpr 0x28ffe28 'const char *'
>
> | `-StringLiteral 0x28ffa68 'const char [2]' lvalue "c"
> `-ImplicitCastExpr 0x28ffe60 'const char *'
>
> `-StringLiteral 0x28ffa98 'const char [2]' lvalue "d"
> ```
>
> For the direct case:
> ```
> const char* list[5] = { "a", "b"};
> ```
>
> It has the following AST-representation:
>
> ```
> VarDecl 0x2c67f00 col:33> col:13 list 'const char *[5]' cinit
> `-InitListExpr 0x2c68010 'const char *[5]'
> |-array filler
> | `-ImplicitValueInitExpr 0x2c68098 <> 'const char *'
> |-ImplicitCastExpr 0x2c68050 'const char *'
> | `-StringLiteral 0x2c67f60 'const char [2]' lvalue "a"
> `-ImplicitCastExpr 0x2c68070 'const char *'
> `-StringLiteral 0x2c67f90 'const char [2]' lvalue "b"
> ```
> So, it seems the length could be taken from the type instead of the
> declaration?!
> Or, look at what is the "Array Filler" (I still don't know). This may be an
> more straight forward way.
>
>
The array filler could be our solution.
I didn't find too much description about it, but based on this:
http://clang.llvm.org/doxygen/Expr_8cpp_source.html#l01963
I guess, there is always an array filler in these cases (when the explicit size
is bigger than the real, so when there is a "hole" in the array because of the
size-diff).
It's more simplier way, you're right.
http://reviews.llvm.org/D19769
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
Re: [PATCH] D19725: [Coverage] Fix an issue where a coverage region might not be created for a macro containing for or while statements.
davidxl accepted this revision.
davidxl added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/CoverageMappingGen.cpp:468
@@ +467,3 @@
+MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation) &&
+isRegionAlreadyAdded(getStartOfFileOrMacro(MostRecentLocation),
+ MostRecentLocation))
Perhaps add a comment here that there is need to create 'outer most' code
region for while/for stmt('s virtual file) with counter equal to the
parent/enclosing scope's counter. The expansion region's counter will be copied
from it during coverage reading. Also refer to handleFileExit where the region
is created.
http://reviews.llvm.org/D19725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
