date:20171030

[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

2017-10-30 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware updated this revision to Diff 120776.
baloghadamsoftware added a comment.

Oops! Sorry!


https://reviews.llvm.org/D39367

Files:
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
  test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp

Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
===
--- test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
@@ -29,3 +29,29 @@
   char *new_name = (char*) std::malloc(non_std::strlen(name + 1));
   // Ignore functions of the strlen family in custom namespaces
 }
+
+void bad_new_strlen(char *name) {
+  char *new_name = new char[std::strlen(name + 1)];
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: Addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = new char\[}}std::strlen(name) + 1{{\];$}}
+}
+
+void good_new_strlen(char *name) {
+  char *new_name = new char[std::strlen(name) + 1];
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:20: warning: Addition operator is applied to the argument of strlen
+}
+
+class C {
+  char c;
+public:
+  static void *operator new[](std::size_t count) {
+return ::operator new(count);
+  }
+};
+
+void bad_custom_new_strlen(char *name) {
+  C *new_name = new C[std::strlen(name + 1)];
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  C \*new_name = new C\[}}std::strlen(name) + 1{{\];$}}
+}
+
Index: docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
===
--- docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
+++ docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
@@ -3,15 +3,16 @@
 bugprone-misplaced-operator-in-strlen-in-alloc
 ==
 
-Finds cases where ``1`` is added to the string in the argument to ``strlen()``,
-``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()``, and ``wcsnlen_s()``
-instead of the result and the value is used as an argument to a memory
-allocation function (``malloc()``, ``calloc()``, ``realloc()``, ``alloca()``).
-Cases where ``1`` is added both to the parameter and the result of the
-``strlen()``-like function are ignored, as are cases where the whole addition is
-surrounded by extra parentheses.
+Finds cases where ``1`` is added to the string in the parameter of ``strlen()``,
+``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and ``wcsnlen_s()``
+functions instead of to the result and use its return value as an argument of a
+memory allocation function (``malloc()``, ``calloc()``, ``realloc()``,
+``alloca()``) or the ``new[]`` operator in `C++`. Cases where ``1`` is added
+both to the parameter and the result of the ``strlen()``-like function are
+ignored, as are cases where the whole addition is surrounded by extra
+parentheses.
 
-Example code:
+`C` example code:
 
 .. code-block:: c
 
@@ -28,7 +29,25 @@
   char *c = (char*) malloc(strlen(str) + 1);
 
 
-Example for silencing the diagnostic:
+`C++` example code:
+
+.. code-block:: c++
+
+void bad_new(char *str) {
+  char *c = new char[strlen(str + 1)];
+}
+
+
+As in the `C` code with the ``malloc()`` function, the suggested fix is to
+add ``1`` to the return value of ``strlen()`` and not to its argument. In the
+example above the fix would be
+
+.. code-block:: c++
+
+  char *c = new char[strlen(str) + 1];
+
+
+Example for silencing the bug report:
 
 .. code-block:: c
 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -60,11 +60,11 @@
 - New `bugprone-misplaced-operator-in-strlen-in-alloc
   `_ check
 
-  Finds cases where ``1`` is added to the string in the argument to
-  ``strlen()``, ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()``, and
-  ``wcsnlen_s()`` instead of the result and the value is used as an argument to
-  a memory allocation function (``malloc()``, ``calloc()``, ``realloc()``,
-  ``alloca()``).
+  Finds cases where ``1`` is added to the string in the parameter of
+  ``strlen()``, ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and
+  ``wcsnlen_s()`` functions instead of to the result and use its return value as
+  an argument of a memory allocation function (``malloc()``, ``calloc()``,
+  ``realloc()``, ``alloca()``) or the ``new[]`` operator in `C++`.
 
 - Renamed checks to use correct term "implicit conversion" instead of "implicit

r316885 - [analyzer] Handle ObjC messages conservatively in CallDescription

2017-10-30 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Mon Oct 30 01:47:13 2017
New Revision: 316885

URL: http://llvm.org/viewvc/llvm-project?rev=316885&view=rev
Log:
[analyzer] Handle ObjC messages conservatively in CallDescription

Differential Revision: https://reviews.llvm.org/D37470

Added:
cfe/trunk/test/Analysis/block-in-critical-section.m
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=316885&r1=316884&r2=316885&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Mon Oct 30 01:47:13 2017
@@ -211,7 +211,9 @@ ProgramPoint CallEvent::getProgramPoint(
 }
 
 bool CallEvent::isCalled(const CallDescription &CD) const {
-  assert(getKind() != CE_ObjCMessage && "Obj-C methods are not supported");
+  // FIXME: Add ObjC Message support.
+  if (getKind() == CE_ObjCMessage)
+return false;
   if (!CD.IsLookupDone) {
 CD.IsLookupDone = true;
 CD.II = 
&getState()->getStateManager().getContext().Idents.get(CD.FuncName);

Added: cfe/trunk/test/Analysis/block-in-critical-section.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/block-in-critical-section.m?rev=316885&view=auto
==
--- cfe/trunk/test/Analysis/block-in-critical-section.m (added)
+++ cfe/trunk/test/Analysis/block-in-critical-section.m Mon Oct 30 01:47:13 2017
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify 
-Wno-objc-root-class %s
+
+@interface SomeClass
+-(void)someMethod;
+@end
+
+void shouldNotCrash(SomeClass *o) {
+  [o someMethod];
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37470: [analyzer] Handle ObjC messages conservatively in CallDescription

2017-10-30 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316885: [analyzer] Handle ObjC messages conservatively in 
CallDescription (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D37470?vs=120598&id=120778#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37470

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
  cfe/trunk/test/Analysis/block-in-critical-section.m


Index: cfe/trunk/test/Analysis/block-in-critical-section.m
===
--- cfe/trunk/test/Analysis/block-in-critical-section.m
+++ cfe/trunk/test/Analysis/block-in-critical-section.m
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify 
-Wno-objc-root-class %s
+
+@interface SomeClass
+-(void)someMethod;
+@end
+
+void shouldNotCrash(SomeClass *o) {
+  [o someMethod];
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -211,7 +211,9 @@
 }
 
 bool CallEvent::isCalled(const CallDescription &CD) const {
-  assert(getKind() != CE_ObjCMessage && "Obj-C methods are not supported");
+  // FIXME: Add ObjC Message support.
+  if (getKind() == CE_ObjCMessage)
+return false;
   if (!CD.IsLookupDone) {
 CD.IsLookupDone = true;
 CD.II = 
&getState()->getStateManager().getContext().Idents.get(CD.FuncName);


Index: cfe/trunk/test/Analysis/block-in-critical-section.m
===
--- cfe/trunk/test/Analysis/block-in-critical-section.m
+++ cfe/trunk/test/Analysis/block-in-critical-section.m
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify -Wno-objc-root-class %s
+
+@interface SomeClass
+-(void)someMethod;
+@end
+
+void shouldNotCrash(SomeClass *o) {
+  [o someMethod];
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -211,7 +211,9 @@
 }
 
 bool CallEvent::isCalled(const CallDescription &CD) const {
-  assert(getKind() != CE_ObjCMessage && "Obj-C methods are not supported");
+  // FIXME: Add ObjC Message support.
+  if (getKind() == CE_ObjCMessage)
+return false;
   if (!CD.IsLookupDone) {
 CD.IsLookupDone = true;
 CD.II = &getState()->getStateManager().getContext().Idents.get(CD.FuncName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316886 - Add missing expected-no-diagnostics comment to test.

2017-10-30 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Mon Oct 30 02:01:48 2017
New Revision: 316886

URL: http://llvm.org/viewvc/llvm-project?rev=316886&view=rev
Log:
Add missing expected-no-diagnostics comment to test.

Modified:
cfe/trunk/test/Analysis/block-in-critical-section.m

Modified: cfe/trunk/test/Analysis/block-in-critical-section.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/block-in-critical-section.m?rev=316886&r1=316885&r2=316886&view=diff
==
--- cfe/trunk/test/Analysis/block-in-critical-section.m (original)
+++ cfe/trunk/test/Analysis/block-in-critical-section.m Mon Oct 30 02:01:48 2017
@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.BlockInCriticalSection -verify 
-Wno-objc-root-class %s
+// expected-no-diagnostics
 
 @interface SomeClass
 -(void)someMethod;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

2017-10-30 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#909255, @aaron.ballman wrote:

> My only remaining concern is with the diagnostic message/fixit interaction 
> itself. Let's see if @alexfh has any suggestions there, or we think of an 
> improvement ourselves.

What do you mean by message/fixit interaction and what is your concern there?

https://reviews.llvm.org/D39121

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316892 - [analyzer] lock_guard and unique_lock extension for BlockInCriticalSection checker

2017-10-30 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Mon Oct 30 03:09:55 2017
New Revision: 316892

URL: http://llvm.org/viewvc/llvm-project?rev=316892&view=rev
Log:
[analyzer] lock_guard and unique_lock extension for BlockInCriticalSection 
checker

A patch by zdtorok (Zoltán Dániel Török)!

Differential Revision: https://reviews.llvm.org/D33729

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
cfe/trunk/test/Analysis/block-in-critical-section.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp?rev=316892&r1=316891&r2=316892&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
(original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp Mon 
Oct 30 03:09:55 2017
@@ -26,15 +26,22 @@ using namespace ento;
 
 namespace {
 
-class BlockInCriticalSectionChecker : public Checker {
+class BlockInCriticalSectionChecker : public Checker {
+
+  mutable IdentifierInfo *IILockGuard, *IIUniqueLock;
 
   CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
   PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
   MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
 
+  StringRef ClassLockGuard, ClassUniqueLock;
+
+  mutable bool IdentifierInfoInitialized;
+
   std::unique_ptr BlockInCritSectionBugType;
 
+  void initIdentifierInfo(ASTContext &Ctx) const;
+
   void reportBlockInCritSection(SymbolRef FileDescSym,
 const CallEvent &call,
 CheckerContext &C) const;
@@ -46,13 +53,10 @@ public:
   bool isLockFunction(const CallEvent &Call) const;
   bool isUnlockFunction(const CallEvent &Call) const;
 
-  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
-
   /// Process unlock.
   /// Process lock.
   /// Process blocking functions (sleep, getc, fgets, read, recv)
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-
 };
 
 } // end anonymous namespace
@@ -60,7 +64,8 @@ public:
 REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
 
 BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
-: LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
+: IILockGuard(nullptr), IIUniqueLock(nullptr),
+  LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
   FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
   PthreadLockFn("pthread_mutex_lock"),
   PthreadTryLockFn("pthread_mutex_trylock"),
@@ -68,13 +73,29 @@ BlockInCriticalSectionChecker::BlockInCr
   MtxLock("mtx_lock"),
   MtxTimedLock("mtx_timedlock"),
   MtxTryLock("mtx_trylock"),
-  MtxUnlock("mtx_unlock") {
+  MtxUnlock("mtx_unlock"),
+  ClassLockGuard("lock_guard"),
+  ClassUniqueLock("unique_lock"),
+  IdentifierInfoInitialized(false) {
   // Initialize the bug type.
   BlockInCritSectionBugType.reset(
   new BugType(this, "Call to blocking function in critical section",
 "Blocking Error"));
 }
 
+void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
+  if (!IdentifierInfoInitialized) {
+/* In case of checking C code, or when the corresponding headers are not
+ * included, we might end up query the identifier table every time when 
this
+ * function is called instead of early returning it. To avoid this, a bool
+ * variable (IdentifierInfoInitialized) is used and the function will be 
run
+ * only once. */
+IILockGuard  = &Ctx.Idents.get(ClassLockGuard);
+IIUniqueLock = &Ctx.Idents.get(ClassUniqueLock);
+IdentifierInfoInitialized = true;
+  }
+}
+
 bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) 
const {
   if (Call.isCalled(SleepFn)
   || Call.isCalled(GetcFn)
@@ -87,6 +108,12 @@ bool BlockInCriticalSectionChecker::isBl
 }
 
 bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) 
const {
+  if (const auto *Ctor = dyn_cast(&Call)) {
+auto IdentifierInfo = Ctor->getDecl()->getParent()->getIdentifier();
+if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
+  return true;
+  }
+
   if (Call.isCalled(LockFn)
   || Call.isCalled(PthreadLockFn)
   || Call.isCalled(PthreadTryLockFn)
@@ -99,6 +126,13 @@ bool BlockInCriticalSectionChecker::isLo
 }
 
 bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) 
const {
+  if (const auto *Dtor = dyn_cast(&Call)) {
+const auto *DRecordDecl = 
dyn_cast(Dtor->getDecl()->getParent());
+auto IdentifierInfo = DRecordDecl->getIdentifier();
+if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
+  return true;
+  }
+
   if (Call.isCalled(UnlockFn)
|| Call.isCalled(PthreadUnlockFn

[PATCH] D39372: Make DiagnosticIDs::getAllDiagnostics static.

2017-10-30 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added a comment.

I like the idea of making functions that can be static, static. Could you 
update the usages of this function as well?


https://reviews.llvm.org/D39372



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

> Does llvm_unreachable() guarantee that the string construction code is 
> completely removed from release builds?

http://llvm.org/docs/CodingStandards.html#assert-liberally:

> When assertions are disabled (i.e. in release builds), `llvm_unreachable` 
> becomes a hint to compilers to skip generating code for this branch. If the 
> compiler does not support this, it will fall back to the “abort” 
> implementation.

So i think it's fine. Not sure how much time it saves compared to writing these 
dumps (as a *lot* of places would need those).


https://reviews.llvm.org/D38986



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

  // TODO: once the constraint manager is smart enough to handle non simplified
  // symbolic expressions remove this function. Note that this can not be used 
in
  // the constraint manager as is, since this does not handle overflows. It is
  // safe to assume, however, that memory offsets will not overflow.

Wasn't safe enough, i guess. This is fairly similar to 
https://reviews.llvm.org/D35109, so someone would have to eventually //do some 
convincing math// to either prove that some sort of "forget about overflows" 
approach is indeed safe, or avoid overflows properly, or handle overflows 
properly. I feel that it's already clear that quick intuition-based solutions 
don't quite cut it when there are a lot of different types, signednesses, 
promotion rules, signed/unsigned overflows, and signed/unsigned extensions 
involved.


Repository:
  rL LLVM

https://reviews.llvm.org/D39049



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39332: [clang-refactor] Introduce "local-qualified-rename" action.

2017-10-30 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 120792.
hokein added a comment.

- rebase to master
- move the implementation to a refactoring rule in `local-rename`
- support non-selection based refactoring action invocation call path


https://reviews.llvm.org/D39332

Files:
  include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  lib/Tooling/Refactoring/RefactoringActions.cpp
  lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  test/Refactor/LocalRename/QualifiedRename.cpp
  tools/clang-refactor/ClangRefactor.cpp

Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -283,10 +283,10 @@
 
   const RefactoringActionRules &getActionRules() const { return ActionRules; }
 
-  /// Parses the command-line arguments that are specific to this rule.
+  /// Parses the "-selection" command-line argument.
   ///
   /// \returns true on error, false otherwise.
-  bool parseArguments() {
+  bool parseSelectionArgument() {
 if (Selection) {
   ParsedSelection = SourceSelectionArgument::fromString(*Selection);
   if (!ParsedSelection)
@@ -464,20 +464,20 @@
 SmallVector MatchingRules;
 llvm::StringSet<> MissingOptions;
 
-bool HasSelection = false;
 for (const auto &Rule : Subcommand.getActionRules()) {
-  bool SelectionMatches = true;
-  if (Rule->hasSelectionRequirement()) {
-HasSelection = true;
-if (!Subcommand.getSelection()) {
-  MissingOptions.insert("selection");
-  SelectionMatches = false;
-}
-  }
   CommandLineRefactoringOptionVisitor Visitor(Subcommand.getOptions());
   Rule->visitRefactoringOptions(Visitor);
-  if (SelectionMatches && Visitor.getMissingRequiredOptions().empty()) {
-MatchingRules.push_back(Rule.get());
+  if (Visitor.getMissingRequiredOptions().empty()) {
+if (!Rule->hasSelectionRequirement()) {
+  MatchingRules.push_back(Rule.get());
+} else {
+  Subcommand.parseSelectionArgument();
+  if (Subcommand.getSelection()) {
+MatchingRules.push_back(Rule.get());
+  } else {
+MissingOptions.insert("selection");
+  }
+}
 continue;
   }
   for (const RefactoringOption *Opt : Visitor.getMissingRequiredOptions())
@@ -490,6 +490,14 @@
 llvm::errs() << "  missing '-" << Opt.getKey() << "' option\n";
   return true;
 }
+if (MatchingRules.size() > 1) {
+  llvm::errs() << "error: find multiple matched action rules for the given "
+  "arguments\n";
+  return true;
+}
+
+RefactoringActionRule* MatchingRule = MatchingRules[0];
+bool HasSelection = MatchingRule->hasSelectionRequirement();
 
 ClangRefactorConsumer Consumer;
 bool HasFailed = false;
@@ -499,16 +507,7 @@
 
   auto InvokeRule = [&](RefactoringResultConsumer &Consumer) {
 logInvocation(Subcommand, Context);
-for (RefactoringActionRule *Rule : MatchingRules) {
-  if (!Rule->hasSelectionRequirement())
-continue;
-  Rule->invoke(Consumer, Context);
-  return;
-}
-// FIXME (Alex L): If more than one initiation succeeded, then the
-// rules are ambiguous.
-llvm_unreachable(
-"The action must have at least one selection rule");
+MatchingRule->invoke(Consumer, Context);
   };
 
   std::unique_ptr CustomConsumer;
@@ -530,7 +529,8 @@
 ActiveConsumer.endTU();
 return;
   }
-  // FIXME (Alex L): Implement non-selection based invocation path.
+
+  InvokeRule(ActiveConsumer);
   ActiveConsumer.endTU();
 }))
   return true;
@@ -567,8 +567,6 @@
   }
   RefactoringActionSubcommand &ActionCommand = **It;
 
-  if (ActionCommand.parseArguments())
-return 1;
   if (Tool.invokeAction(ActionCommand, Options.getCompilations(),
 Options.getSourcePathList()))
 return 1;
Index: test/Refactor/LocalRename/QualifiedRename.cpp
===
--- /dev/null
+++ test/Refactor/LocalRename/QualifiedRename.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-refactor local-rename -old-qualified-name="foo::A" -new-qualified-name="bar::B" %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck %s
+
+namespace foo {
+class A {};
+}
+// CHECK: namespace foo {
+// CHECK-NEXT: class B {};
+// CHECK-NEXT: }
+
+namespace bar {
+void f(foo::A* a) {
+  foo::A b;
+}
+// CHECK: void f(B* a) {
+// CHECK-NEXT:   B b;
+// CHECK-NEXT: }
+}
+
+void f(foo::A* a) {
+  foo::A b;
+}
+// CHECK: void f(bar::B* a) {
+// CHECK-NEXT:   bar::B b;
+// CHECK-NEXT: }
Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- lib/Tooli

[PATCH] D39332: [clang-refactor] Introduce "local-qualified-rename" action.

2017-10-30 Thread Haojian Wu via Phabricator via cfe-commits

hokein marked an inline comment as done.
hokein added inline comments.



Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:

ioeric wrote:
> arphaman wrote:
> > ioeric wrote:
> > > ioeric wrote:
> > > > arphaman wrote:
> > > > > hokein wrote:
> > > > > > sammccall wrote:
> > > > > > > As discussed offline, it's not clear why this is a separate 
> > > > > > > Action, rather than a different Rule that's part of the same 
> > > > > > > Action.
> > > > > > > 
> > > > > > > @arphaman how does the framework answer this question?
> > > > > > There is a 
> > > > > > [document](https://clang.llvm.org/docs/RefactoringEngine.html#refactoring-action-rules)
> > > > > >  describing it, but still ambiguous.
> > > > > > 
> > > > > > We also had some questions about `local-rename` from the 
> > > > > > discussion, need @arphaman's input:
> > > > > > 
> > > > > > * `OccurrenceFinder` is not exposed now, it is merely used in 
> > > > > > `RenameOccurrences`. We think there should be a public interface to 
> > > > > > the clients, like for implementing interactive mode in IDE? 
> > > > > > * Currently the rules defined in the same action must have mutual 
> > > > > > command-line options, otherwise clang-refactor would complain the 
> > > > > > command-line option are being registered more than once. It might 
> > > > > > be very strict for some cases. For example, `-new-name` is most 
> > > > > > likely being used by many rules in `local-rename` action.
> > > > > >  
> > > > > I think that this should be just a rule in `local-rename`.
> > > > > 
> > > > > So you'd be able to call:
> > > > > 
> > > > > `clang-refactor local-rename -selection=X -new-name=foo`
> > > > > `clang-refactor local-rename -old-qualified-name=bar -new-name=foo`.
> > > > We need your help to understand how exactly `local-rename` is intended 
> > > > to be used. 
> > > > 
> > > > From the current code and previous conversations we had, it seems to me 
> > > > that the action would support the use case where a user selects an 
> > > > identifier in the editor (say, with cursor) and initiates a 
> > > > `local-rename` action but without providing the new name in the 
> > > > beginning. The rename rule finds and returns all occurrences (including 
> > > > token ranges)  to the editor, and users can then start typing in the 
> > > > new name, and in the same time, the editor performs text replacements 
> > > > according to ranges of occurrences and the new name typed in. Is this 
> > > > how `RenameOccurrences` is intended to be used in the future? 
> > > > 
> > > > If this is how `local-rename` is expected to be used, it would be hard 
> > > > to merge qualified rename into it, because both qualified old name and 
> > > > new name are required in order to calculate the range of a symbol 
> > > > reference, and this doesn't fit with the above workflow. But if my 
> > > > understanding is simply wrong (e.g. the editor would invoke 
> > > > `local-rename` again to perform the actual refactoring), then I think 
> > > > it makes a lot of sense to merge qualified rename into the current 
> > > > local-rename action.
> > > Sorry, by "your help", I was referring to Alex ;) @arphaman 
> > You're right that rename should deal with occurrences conceptually, but I 
> > believe that's more of requirement imposed onto the editor clients. Rename 
> > in particular is basically impossible to map to all clients using just one 
> > generic model, so I think it's fine if `RenameOccurrences` class returns 
> > source replacements that `local-rename` in `clang-refactor` consumes. I 
> > don't think this will change in the future, if anything we will lift 
> > `OccurrenceFinder`class into the header so that the editor clients can use 
> > it.
> > So I think in terms of the tool it should be ok to have immediate 
> > `local-rename` action that behaves similarly to `clang-rename` and deals 
> > with source changes and not replacements.
> Thanks for the clarification! In that case, I agree that qualified rename 
> should be a rule in the local-rename action.
Thanks for the explanation, that makes sense. Moved to `local-rename` action.


https://reviews.llvm.org/D39332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type.

2017-10-30 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added a comment.

I like the idea of making `ProgramState::getSVal(const MemRegion *)` symmetric 
to `ProgramState::getSVal(Loc)`.

Just some philosophical questions which should probably be addressed in the 
future:
But also I wonder, should we maintain all these overloads? I mean, a lot of the 
APIs accept both `SVal`/`Loc` and `MemRegion *`. Usually, one of the 
implementations will end up just calling the other after some 
wrapping/unwrapping.  Maybe ripping `MemRegion *` from the APIs would make it 
easier to use? E.g.: CallAndMessageChecker just unwraps the region from an SVal 
just to wrap it back again within the API call. However, we tend to use 
`MemRegion *` when storing info in the program state.

Otherwise LGTM!




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:307
   /// Returns the "raw" SVal bound to LV before any value simplfication.
   SVal getRawSVal(Loc LV, QualType T= QualType()) const;
 

The whitespace here is a bit off. I know it is not related to this patch, but 
this could be fixed if we already touch this file.


https://reviews.llvm.org/D38801



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316895 - [refactor] Fix a clang-tidy warning.

2017-10-30 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Oct 30 04:17:09 2017
New Revision: 316895

URL: http://llvm.org/viewvc/llvm-project?rev=316895&view=rev
Log:
[refactor] Fix a clang-tidy warning.

NFC

Modified:
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp?rev=316895&r1=316894&r2=316895&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Mon Oct 30 
04:17:09 2017
@@ -77,7 +77,8 @@ RenameOccurrences::initiate(RefactoringR
   if (!ND)
 return Context.createDiagnosticError(
 SelectionRange.getBegin(), diag::err_refactor_selection_no_symbol);
-  return RenameOccurrences(getCanonicalSymbolDeclaration(ND), NewName);
+  return RenameOccurrences(getCanonicalSymbolDeclaration(ND),
+   std::move(NewName));
 }
 
 Expected


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-10-30 Thread John Baldwin via Phabricator via cfe-commits

bsdjhb updated this revision to Diff 120798.
bsdjhb marked an inline comment as done.
bsdjhb added a comment.

- Rebase on more MAX_REGISTER changes.
- Use macro for lastDwarfRegisterNumber.
- Move MIPS ABI constants under a single #ifdef __mips__.


https://reviews.llvm.org/D38110

Files:
  include/__libunwind_config.h
  include/libunwind.h
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/UnwindRegistersRestore.S
  src/UnwindRegistersSave.S
  src/config.h
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -59,8 +59,12 @@
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_o32
+#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_n64
 #elif defined(__mips__)
-# warning The MIPS architecture is not supported.
+# warning The MIPS architecture is not supported with this ABI and environment!
 #else
 # error Architecture not supported
 #endif
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -67,7 +67,7 @@
 defined(__ppc__) || defined(__ppc64__) ||  \
 (!defined(__APPLE__) && defined(__arm__)) ||   \
 (defined(__arm64__) || defined(__aarch64__)) ||\
-(defined(__APPLE__) && defined(__mips__))
+defined(__mips__)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
 
Index: src/UnwindRegistersSave.S
===
--- src/UnwindRegistersSave.S
+++ src/UnwindRegistersSave.S
@@ -116,6 +116,118 @@
   xorl  %eax, %eax# return UNW_ESUCCESS
   ret
 
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+
+#
+# extern int unw_getcontext(unw_context_t* thread_state)
+#
+# On entry:
+#  thread_state pointer is in a0 ($4)
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  sw$1, (4 * 1)($4)
+  sw$2, (4 * 2)($4)
+  sw$3, (4 * 3)($4)
+  sw$4, (4 * 4)($4)
+  sw$5, (4 * 5)($4)
+  sw$6, (4 * 6)($4)
+  sw$7, (4 * 7)($4)
+  sw$8, (4 * 8)($4)
+  sw$9, (4 * 9)($4)
+  sw$10, (4 * 10)($4)
+  sw$11, (4 * 11)($4)
+  sw$12, (4 * 12)($4)
+  sw$13, (4 * 13)($4)
+  sw$14, (4 * 14)($4)
+  sw$15, (4 * 15)($4)
+  sw$16, (4 * 16)($4)
+  sw$17, (4 * 17)($4)
+  sw$18, (4 * 18)($4)
+  sw$19, (4 * 19)($4)
+  sw$20, (4 * 20)($4)
+  sw$21, (4 * 21)($4)
+  sw$22, (4 * 22)($4)
+  sw$23, (4 * 23)($4)
+  sw$24, (4 * 24)($4)
+  sw$25, (4 * 25)($4)
+  sw$26, (4 * 26)($4)
+  sw$27, (4 * 27)($4)
+  sw$28, (4 * 28)($4)
+  sw$29, (4 * 29)($4)
+  sw$30, (4 * 30)($4)
+  sw$31, (4 * 31)($4)
+  # Store return address to pc
+  sw$31, (4 * 32)($4)
+  # hi and lo
+  mfhi  $8
+  sw$8,  (4 * 33)($4)
+  mflo  $8
+  sw$8,  (4 * 34)($4)
+  jr	$31
+  # return UNW_ESUCCESS
+  or$2, $0, $0
+  .set pop
+
+#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+
+#
+# extern int unw_getcontext(unw_context_t* thread_state)
+#
+# On entry:
+#  thread_state pointer is in a0 ($4)
+#
+DEFINE_LIBUNWIND_FUNCTION(unw_getcontext)
+  .set push
+  .set noat
+  .set noreorder
+  .set nomacro
+  sd$1, (8 * 1)($4)
+  sd$2, (8 * 2)($4)
+  sd$3, (8 * 3)($4)
+  sd$4, (8 * 4)($4)
+  sd$5, (8 * 5)($4)
+  sd$6, (8 * 6)($4)
+  sd$7, (8 * 7)($4)
+  sd$8, (8 * 8)($4)
+  sd$9, (8 * 9)($4)
+  sd$10, (8 * 10)($4)
+  sd$11, (8 * 11)($4)
+  sd$12, (8 * 12)($4)
+  sd$13, (8 * 13)($4)
+  sd$14, (8 * 14)($4)
+  sd$15, (8 * 15)($4)
+  sd$16, (8 * 16)($4)
+  sd$17, (8 * 17)($4)
+  sd$18, (8 * 18)($4)
+  sd$19, (8 * 19)($4)
+  sd$20, (8 * 20)($4)
+  sd$21, (8 * 21)($4)
+  sd$22, (8 * 22)($4)
+  sd$23, (8 * 23)($4)
+  sd$24, (8 * 24)($4)
+  sd$25, (8 * 25)($4)
+  sd$26, (8 * 26)($4)
+  sd$27, (8 * 27)($4)
+  sd$28, (8 * 28)($4)
+  sd$29, (8 * 29)($4)
+  sd$30, (8 * 30)($4)
+  sd$31, (8 * 31)($4)
+  # Store return address to pc
+  sd$31, (8 * 32)($4)
+  # hi and lo
+  mfhi  $8
+  sd$8,  (8 * 33)($4)
+  mflo  $8
+  sd$8,  (8 * 34)($4)
+  jr	$31
+  # return UNW_ESUCCESS
+  or$2, $0, $0
+  .set pop
+
 # elif defined(__mips__)
 
 #
Index: src/UnwindRegistersRestore.S
===
--- src/UnwindRegistersRestore.S
+++ src/UnwindRegistersRestore.S
@@ -524,6 +524,118 @@
   l.jr r9
l.nop
 
+#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+
+//
+// void libunwind::Registers_mips_o32::jumpto()
+//
+// On entr

[PATCH] D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type.

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

Yeah, cleaning up this API would be great - as long as everybody loves to have 
the API broken and rewrite stuff.

If we are to simplify some APIs, i'd definitely start with 
`getAsSymExpr()`/`getAsSymbol()`/`getAsSymbolicExpression()`.

Essentially we only need `getSVal(MR)` because only `MR` can be bound to, while 
other types of `Loc` cannot be bound to. Technically, only (base `MR`, offset) 
pairs can be bound to, but this is "implementation detail" that we're still 
trying to keep that way (even though it leaks like hell everywhere). So i'd 
rather think of the `Loc` overload as of a convenient wrapper (i.e., the user 
receives `Loc` in some `checkLocation` callback, so he wants to put it directly 
into the API to find his binding). And people would still need to operate with 
regions from time to time (eg., construct a sub-region manually when we expect 
it to be there, or strip casts).

Also the overload between `getSVal(Ex, LC)` and `getSVal(R, Ty)`, when they do 
completely different things, seems confusing.


https://reviews.llvm.org/D38801



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

Just a heads up WRT this patch; we're discussing changing the size of 
`unw_word_t` to match `uintptr_t` in https://reviews.llvm.org/D39365. Does that 
break anything for your case? It shouldn't affect what's stored in the Register 
class, only pointers in the unw_proc_info_t struct. Not sure which patch will 
get completed/merged first though.


https://reviews.llvm.org/D38110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

Yay.

Thanks Gabor, i didn't think about it but it look very nice to have such 
matcher.

I think the matcher shouldn't be checker-local, but shared into `ASTMatchers.h` 
all other matchers - not only this matcher is universally useful, but also i 
have weird vague memories that matchers may collide by name if defined in 
different translation units differently (if this is indeed this way then having 
the same matcher in multiple places is very bad).


https://reviews.llvm.org/D38921



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316896 - [CodeGen] Generate TBAA info for reference loads

2017-10-30 Thread Ivan A. Kosarev via cfe-commits

Author: kosarev
Date: Mon Oct 30 04:49:31 2017
New Revision: 316896

URL: http://llvm.org/viewvc/llvm-project?rev=316896&view=rev
Log:
[CodeGen] Generate TBAA info for reference loads

Differential Revision: https://reviews.llvm.org/D39177

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/tbaa-reference.cpp

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=316896&r1=316895&r2=316896&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Mon Oct 30 04:49:31 2017
@@ -1189,8 +1189,8 @@ Address CodeGenFunction::GetAddrOfBlockD
  variable->getName());
   }
 
-  if (auto refType = capture.fieldType()->getAs())
-addr = EmitLoadOfReference(addr, refType);
+  if (capture.fieldType()->isReferenceType())
+addr = EmitLoadOfReference(MakeAddrLValue(addr, capture.fieldType()));
 
   return addr;
 }

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=316896&r1=316895&r2=316896&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Oct 30 04:49:31 2017
@@ -2160,22 +2160,30 @@ static LValue EmitThreadPrivateVarDeclLV
   return CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
 }
 
-Address CodeGenFunction::EmitLoadOfReference(Address Addr,
- const ReferenceType *RefTy,
- LValueBaseInfo *BaseInfo,
- TBAAAccessInfo *TBAAInfo) {
-  llvm::Value *Ptr = Builder.CreateLoad(Addr);
-  return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
-  BaseInfo, TBAAInfo,
-  /* forPointeeType= */ true));
-}
-
-LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,
-  const ReferenceType *RefTy) {
-  LValueBaseInfo BaseInfo;
-  TBAAAccessInfo TBAAInfo;
-  Address Addr = EmitLoadOfReference(RefAddr, RefTy, &BaseInfo, &TBAAInfo);
-  return MakeAddrLValue(Addr, RefTy->getPointeeType(), BaseInfo, TBAAInfo);
+Address
+CodeGenFunction::EmitLoadOfReference(LValue RefLVal,
+ LValueBaseInfo *PointeeBaseInfo,
+ TBAAAccessInfo *PointeeTBAAInfo) {
+  llvm::LoadInst *Load = Builder.CreateLoad(RefLVal.getAddress(),
+RefLVal.isVolatile());
+  TBAAAccessInfo RefTBAAInfo = RefLVal.getTBAAInfo();
+  if (RefLVal.getBaseInfo().getMayAlias())
+RefTBAAInfo = CGM.getTBAAMayAliasAccessInfo();
+  CGM.DecorateInstructionWithTBAA(Load, RefTBAAInfo);
+
+  CharUnits Align = 
getNaturalTypeAlignment(RefLVal.getType()->getPointeeType(),
+PointeeBaseInfo, PointeeTBAAInfo,
+/* forPointeeType= */ true);
+  return Address(Load, Align);
+}
+
+LValue CodeGenFunction::EmitLoadOfReferenceLValue(LValue RefLVal) {
+  LValueBaseInfo PointeeBaseInfo;
+  TBAAAccessInfo PointeeTBAAInfo;
+  Address PointeeAddr = EmitLoadOfReference(RefLVal, &PointeeBaseInfo,
+&PointeeTBAAInfo);
+  return MakeAddrLValue(PointeeAddr, RefLVal.getType()->getPointeeType(),
+PointeeBaseInfo, PointeeTBAAInfo);
 }
 
 Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
@@ -2210,17 +2218,15 @@ static LValue EmitGlobalVarDeclLValue(Co
   V = EmitBitCastOfLValueToProperType(CGF, V, RealVarTy);
   CharUnits Alignment = CGF.getContext().getDeclAlign(VD);
   Address Addr(V, Alignment);
-  LValue LV;
   // Emit reference to the private copy of the variable if it is an OpenMP
   // threadprivate variable.
   if (CGF.getLangOpts().OpenMP && VD->hasAttr())
 return EmitThreadPrivateVarDeclLValue(CGF, VD, T, Addr, RealVarTy,
   E->getExprLoc());
-  if (auto RefTy = VD->getType()->getAs()) {
-LV = CGF.EmitLoadOfReferenceLValue(Addr, RefTy);
-  } else {
-LV = CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
-  }
+  LValue LV = VD->getType()->isReferenceType() ?
+  CGF.EmitLoadOfReferenceLValue(Addr, VD->getType(),
+AlignmentSource::Decl) :
+  CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
   setObjCGCLValueClass(CGF.getContext(), E, LV);
   return LV;
 }
@@ -2338,8 +2344,9 @@ LValue CodeGenFunction::EmitDeclRefLValu
   else if (CapturedSt

[PATCH] D39177: [CodeGen] Generate TBAA info for reference loads

2017-10-30 Thread Ivan Kosarev via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316896: [CodeGen] Generate TBAA info for reference loads 
(authored by kosarev).

Changed prior to commit:
  https://reviews.llvm.org/D39177?vs=120654&id=120800#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39177

Files:
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGen/tbaa-reference.cpp

Index: cfe/trunk/test/CodeGen/tbaa-reference.cpp
===
--- cfe/trunk/test/CodeGen/tbaa-reference.cpp
+++ cfe/trunk/test/CodeGen/tbaa-reference.cpp
@@ -6,24 +6,32 @@
 
 struct B {
   S &s;
-  B(S &s) : s(s) {}
-  void bar();
+  B(S &s);
+  S &get();
 };
 
-void foo(S &s) {
-  B b(s);
-  b.bar();
-}
-
-// CHECK-LABEL: _Z3fooR1S
-// Check initialization of the reference parameter in foo().
-// CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer:!.*]]
-//
+B::B(S &s) : s(s) {
 // CHECK-LABEL: _ZN1BC2ER1S
-// TODO: Check loading of the reference parameter in B::B(S&).
-// Check initialization of B::s in B::B(S&).
+// Check initialization of the reference parameter.
+// CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer:!.*]]
+
+// Check loading of the reference parameter.
+// CHECK: load %struct.S*, %struct.S** {{.*}}, !tbaa [[TAG_pointer]]
+
+// Check initialization of the reference member.
 // CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer]]
-//
+}
+
+S &B::get() {
+// CHECK-LABEL: _ZN1B3getEv
+// Check that we access the reference as a structure member.
+// CHECK: load %struct.S*, %struct.S** {{.*}}, !tbaa [[TAG_B_s:!.*]]
+  return s;
+}
+
 // CHECK-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0}
+// CHECK-DAG: [[TAG_B_s]] = !{[[TYPE_B:!.*]], [[TYPE_pointer]], i64 0}
+//
+// CHECK-DAG: [[TYPE_B]] = !{!"_ZTS1B", [[TYPE_pointer]], i64 0}
 // CHECK-DAG: [[TYPE_pointer]] = !{!"any pointer", [[TYPE_char:!.*]], i64 0}
 // CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{!.*}}, i64 0}
Index: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
@@ -417,8 +417,7 @@
   Address ArgAddr = ArgLVal.getAddress();
   if (!VarTy->isReferenceType()) {
 if (ArgLVal.getType()->isLValueReferenceType()) {
-  ArgAddr = CGF.EmitLoadOfReference(
-  ArgAddr, ArgLVal.getType()->castAs());
+  ArgAddr = CGF.EmitLoadOfReference(ArgLVal);
 } else if (!VarTy->isVariablyModifiedType() || !VarTy->isPointerType()) {
   assert(ArgLVal.getType()->isPointerType());
   ArgAddr = CGF.EmitLoadOfPointer(
Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1037,8 +1037,8 @@
 if (auto *PtrTy = BaseTy->getAs())
   BaseLV = CGF.EmitLoadOfPointerLValue(BaseLV.getAddress(), PtrTy);
 else {
-  BaseLV = CGF.EmitLoadOfReferenceLValue(BaseLV.getAddress(),
- BaseTy->castAs());
+  LValue RefLVal = CGF.MakeAddrLValue(BaseLV.getAddress(), BaseTy);
+  BaseLV = CGF.EmitLoadOfReferenceLValue(RefLVal);
 }
 BaseTy = BaseTy->getPointeeType();
   }
Index: cfe/trunk/lib/CodeGen/CGBlocks.cpp
===
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp
@@ -1189,8 +1189,8 @@
  variable->getName());
   }
 
-  if (auto refType = capture.fieldType()->getAs())
-addr = EmitLoadOfReference(addr, refType);
+  if (capture.fieldType()->isReferenceType())
+addr = EmitLoadOfReference(MakeAddrLValue(addr, capture.fieldType()));
 
   return addr;
 }
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2160,22 +2160,30 @@
   return CGF.MakeAddrLValue(Addr, T, AlignmentSource::Decl);
 }
 
-Address CodeGenFunction::EmitLoadOfReference(Address Addr,
- const ReferenceType *RefTy,
- LValueBaseInfo *BaseInfo,
- TBAAAccessInfo *TBAAInfo) {
-  llvm::Value *Ptr = Builder.CreateLoad(Addr);
-  return Address(Ptr, getNaturalTypeAlignment(RefTy->getPointeeType(),
-  BaseInfo, TBAAInfo,
-  /* forPointeeType= */ true));
-}
-
-LValue CodeGenFunction::EmitLoadOfReferenceLValue(Address RefAddr,

[PATCH] D38844: [analyzer] Make issue hash related tests more concise

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great stuff!




Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:84
   &ExprInspectionChecker::analyzerNumTimesReached)
+.Case("clang_analyzer_hashDump", &ExprInspectionChecker::analyzerDumpHash)
 .Default(nullptr);

`hashDump` <=> `DumpHash` (looks slightly typo-ish)



Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:288-295
+const LangOptions &Opts = C.getLangOpts();
+const SourceManager &SM = C.getSourceManager();
+FullSourceLoc FL(CE->getArg(0)->getLocStart(), SM);
+std::string HashContent =
+GetIssueString(SM, FL, getCheckName().getName(), "Category",
+   C.getLocationContext()->getDecl(), Opts);
+

Accidentally 4 spaces here.


https://reviews.llvm.org/D38844



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

2017-10-30 Thread John Baldwin via Phabricator via cfe-commits

bsdjhb added a comment.

In https://reviews.llvm.org/D38110#910526, @mstorsjo wrote:

> Just a heads up WRT this patch; we're discussing changing the size of 
> `unw_word_t` to match `uintptr_t` in https://reviews.llvm.org/D39365. Does 
> that break anything for your case? It shouldn't affect what's stored in the 
> Register class, only pointers in the unw_proc_info_t struct. Not sure which 
> patch will get completed/merged first though.

Yes, I saw that.  It will probably cause some breakage for the cursor size 
depending on which path (uint64_t always vs uintptr_t).  I'm not quite sure 
which of those approaches is more correct to be honest.  I also have an N32 
patch in review but will wait until this one is finally committed before 
updating that further.

https://reviews.llvm.org/D38110

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316900 - [analyzer] Use the signature of the primary template for issue hash calculation

2017-10-30 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Mon Oct 30 05:16:07 2017
New Revision: 316900

URL: http://llvm.org/viewvc/llvm-project?rev=316900&view=rev
Log:
[analyzer] Use the signature of the primary template for issue hash calculation

Now when a template is instantiated more times and there is a bug found in the
instantiations the issue hash will be different for each instantiation even if
every other property of the bug (path, message, location) is the same.

This patch aims to resolve this issue. Note that explicit specializations still
generate different hashes but that is intended.

Differential Revision: https://reviews.llvm.org/D38728

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
cfe/trunk/test/Analysis/bug_hash_test.cpp
cfe/trunk/test/Analysis/edges-new.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp?rev=316900&r1=316899&r2=316900&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp Mon Oct 30 05:16:07 2017
@@ -33,6 +33,13 @@ static std::string GetSignature(const Fu
 return "";
   std::string Signature;
 
+  // When a flow sensitive bug happens in templated code we should not generate
+  // distinct hash value for every instantiation. Use the signature from the
+  // primary template.
+  if (const FunctionDecl *InstantiatedFrom =
+  Target->getTemplateInstantiationPattern())
+Target = InstantiatedFrom;
+
   if (!isa(Target) && !isa(Target) &&
   !isa(Target))
 Signature.append(Target->getReturnType().getAsString()).append(" ");

Modified: cfe/trunk/test/Analysis/bug_hash_test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bug_hash_test.cpp?rev=316900&r1=316899&r2=316900&view=diff
==
--- cfe/trunk/test/Analysis/bug_hash_test.cpp (original)
+++ cfe/trunk/test/Analysis/bug_hash_test.cpp Mon Oct 30 05:16:07 2017
@@ -71,15 +71,13 @@ void testLambda() {
 
 template 
 void f(T) {
-  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void 
f(double)$27$clang_analyzer_hashDump(5);$Category}}
-   // 
expected-warning@-1{{debug.ExprInspection$void 
f(int)$27$clang_analyzer_hashDump(5);$Category}}
+  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void 
f(T)$27$clang_analyzer_hashDump(5);$Category}}
 }
 
 template 
 struct TX {
   void f(T) {
-clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void 
TX::f(double)$29$clang_analyzer_hashDump(5);$Category}}
- // 
expected-warning@-1{{debug.ExprInspection$void 
TX::f(int)$29$clang_analyzer_hashDump(5);$Category}}
+clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TX::f(T)$29$clang_analyzer_hashDump(5);$Category}}
   }
 };
 
@@ -99,11 +97,17 @@ template 
 struct TTX {
   template
   void f(T, S) {
-clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TTX::f(int, 
int)$29$clang_analyzer_hashDump(5);$Category}}
+clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TTX::f(T, 
S)$29$clang_analyzer_hashDump(5);$Category}}
   }
 };
 
 void g() {
+  // TX and TX is instantiated from the same code with the same
+  // source locations. The same error happining in both of the instantiations
+  // should share the common hash. This means we should not include the
+  // template argument for these types in the function signature.
+  // Note that, we still want the hash to be different for explicit
+  // specializations.
   TX x;
   TX y;
   TX xl;

Modified: cfe/trunk/test/Analysis/edges-new.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/edges-new.mm?rev=316900&r1=316899&r2=316900&view=diff
==
--- cfe/trunk/test/Analysis/edges-new.mm (original)
+++ cfe/trunk/test/Analysis/edges-new.mm Mon Oct 30 05:16:07 2017
@@ -20288,7 +20288,7 @@ namespace rdar14960554 {
 // CHECK-NEXT:typeBad deallocator
 // CHECK-NEXT:
check_nameunix.MismatchedDeallocator
 // CHECK-NEXT:
-// CHECK-NEXT:
issue_hash_content_of_line_in_contextd9dbbf68db41ab74e2158f4b131abe34
+// CHECK-NEXT:
issue_hash_content_of_line_in_context046c88d1c91ff46d6506dff5ff880756
 // CHECK-NEXT:   issue_hash_function_offset0
 // CHECK-NEXT:   location
 // CHECK-NEXT:   


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

2017-10-30 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316900: [analyzer] Use the signature of the primary template 
for issue hash calculation (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D38728?vs=118788&id=120804#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38728

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
  cfe/trunk/test/Analysis/bug_hash_test.cpp
  cfe/trunk/test/Analysis/edges-new.mm


Index: cfe/trunk/test/Analysis/bug_hash_test.cpp
===
--- cfe/trunk/test/Analysis/bug_hash_test.cpp
+++ cfe/trunk/test/Analysis/bug_hash_test.cpp
@@ -71,15 +71,13 @@
 
 template 
 void f(T) {
-  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void 
f(double)$27$clang_analyzer_hashDump(5);$Category}}
-   // 
expected-warning@-1{{debug.ExprInspection$void 
f(int)$27$clang_analyzer_hashDump(5);$Category}}
+  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void 
f(T)$27$clang_analyzer_hashDump(5);$Category}}
 }
 
 template 
 struct TX {
   void f(T) {
-clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void 
TX::f(double)$29$clang_analyzer_hashDump(5);$Category}}
- // 
expected-warning@-1{{debug.ExprInspection$void 
TX::f(int)$29$clang_analyzer_hashDump(5);$Category}}
+clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TX::f(T)$29$clang_analyzer_hashDump(5);$Category}}
   }
 };
 
@@ -99,11 +97,17 @@
 struct TTX {
   template
   void f(T, S) {
-clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TTX::f(int, 
int)$29$clang_analyzer_hashDump(5);$Category}}
+clang_analyzer_hashDump(5); // expected-warning 
{{debug.ExprInspection$void TTX::f(T, 
S)$29$clang_analyzer_hashDump(5);$Category}}
   }
 };
 
 void g() {
+  // TX and TX is instantiated from the same code with the same
+  // source locations. The same error happining in both of the instantiations
+  // should share the common hash. This means we should not include the
+  // template argument for these types in the function signature.
+  // Note that, we still want the hash to be different for explicit
+  // specializations.
   TX x;
   TX y;
   TX xl;
Index: cfe/trunk/test/Analysis/edges-new.mm
===
--- cfe/trunk/test/Analysis/edges-new.mm
+++ cfe/trunk/test/Analysis/edges-new.mm
@@ -20288,7 +20288,7 @@
 // CHECK-NEXT:typeBad deallocator
 // CHECK-NEXT:
check_nameunix.MismatchedDeallocator
 // CHECK-NEXT:
-// CHECK-NEXT:
issue_hash_content_of_line_in_contextd9dbbf68db41ab74e2158f4b131abe34
+// CHECK-NEXT:
issue_hash_content_of_line_in_context046c88d1c91ff46d6506dff5ff880756
 // CHECK-NEXT:   issue_hash_function_offset0
 // CHECK-NEXT:   location
 // CHECK-NEXT:   
Index: cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
@@ -33,6 +33,13 @@
 return "";
   std::string Signature;
 
+  // When a flow sensitive bug happens in templated code we should not generate
+  // distinct hash value for every instantiation. Use the signature from the
+  // primary template.
+  if (const FunctionDecl *InstantiatedFrom =
+  Target->getTemplateInstantiationPattern())
+Target = InstantiatedFrom;
+
   if (!isa(Target) && !isa(Target) &&
   !isa(Target))
 Signature.append(Target->getReturnType().getAsString()).append(" ");


Index: cfe/trunk/test/Analysis/bug_hash_test.cpp
===
--- cfe/trunk/test/Analysis/bug_hash_test.cpp
+++ cfe/trunk/test/Analysis/bug_hash_test.cpp
@@ -71,15 +71,13 @@
 
 template 
 void f(T) {
-  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void f(double)$27$clang_analyzer_hashDump(5);$Category}}
-   // expected-warning@-1{{debug.ExprInspection$void f(int)$27$clang_analyzer_hashDump(5);$Category}}
+  clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void f(T)$27$clang_analyzer_hashDump(5);$Category}}
 }
 
 template 
 struct TX {
   void f(T) {
-clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void TX::f(double)$29$clang_analyzer_hashDump(5);$Category}}
- // expected-warning@-1{{debug.ExprInspection$void TX::f(int)$29$clang_analyzer_hashDump(5);$Category}}
+clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void TX::f(T)$29$clang_analyzer_hashDump(5);$Category}}
   }
 };
 
@@ -99,11 +97,17 @@
 struct TTX {
   template
   void f(T, S) {
-clang_analyzer_hashDump(5); // expected-warning {{debug.ExprInspection$void TTX::f(int, int)$29$clang_analy

[PATCH] D39416: [modules] Correctly overload getModule in the MultiplexExternalSemaSource

2017-10-30 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor created this revision.

The MultiplexExternalSemaSource doesn't correctly overload the `getModule` 
function,
causing the multiplexer to not forward this call as intended.


https://reviews.llvm.org/D39416

Files:
  include/clang/Sema/MultiplexExternalSemaSource.h
  lib/Sema/MultiplexExternalSemaSource.cpp


Index: lib/Sema/MultiplexExternalSemaSource.cpp
===
--- lib/Sema/MultiplexExternalSemaSource.cpp
+++ lib/Sema/MultiplexExternalSemaSource.cpp
@@ -164,6 +164,13 @@
 Sources[i]->PrintStats();
 }
 
+Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+if (auto M = Sources[i]->getModule(ID))
+  return M;
+  return nullptr;
+}
+
 bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
uint64_t &Size, 
uint64_t &Alignment,
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===
--- include/clang/Sema/MultiplexExternalSemaSource.h
+++ include/clang/Sema/MultiplexExternalSemaSource.h
@@ -148,8 +148,10 @@
   /// \brief Print any statistics that have been gathered regarding
   /// the external AST source.
   void PrintStats() override;
-  
-  
+
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  virtual Module *getModule(unsigned ID) override;
+
   /// \brief Perform layout on the given record.
   ///
   /// This routine allows the external AST source to provide an specific 


Index: lib/Sema/MultiplexExternalSemaSource.cpp
===
--- lib/Sema/MultiplexExternalSemaSource.cpp
+++ lib/Sema/MultiplexExternalSemaSource.cpp
@@ -164,6 +164,13 @@
 Sources[i]->PrintStats();
 }
 
+Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+if (auto M = Sources[i]->getModule(ID))
+  return M;
+  return nullptr;
+}
+
 bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
uint64_t &Size, 
uint64_t &Alignment,
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===
--- include/clang/Sema/MultiplexExternalSemaSource.h
+++ include/clang/Sema/MultiplexExternalSemaSource.h
@@ -148,8 +148,10 @@
   /// \brief Print any statistics that have been gathered regarding
   /// the external AST source.
   void PrintStats() override;
-  
-  
+
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  virtual Module *getModule(unsigned ID) override;
+
   /// \brief Perform layout on the given record.
   ///
   /// This routine allows the external AST source to provide an specific 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39416: [modules] Correctly overload getModule in the MultiplexExternalSemaSource

2017-10-30 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor updated this revision to Diff 120808.
teemperor added a comment.

- Removed `virtual` keyword when we already have `override`.


https://reviews.llvm.org/D39416

Files:
  include/clang/Sema/MultiplexExternalSemaSource.h
  lib/Sema/MultiplexExternalSemaSource.cpp


Index: lib/Sema/MultiplexExternalSemaSource.cpp
===
--- lib/Sema/MultiplexExternalSemaSource.cpp
+++ lib/Sema/MultiplexExternalSemaSource.cpp
@@ -164,6 +164,13 @@
 Sources[i]->PrintStats();
 }
 
+Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+if (auto M = Sources[i]->getModule(ID))
+  return M;
+  return nullptr;
+}
+
 bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
uint64_t &Size, 
uint64_t &Alignment,
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===
--- include/clang/Sema/MultiplexExternalSemaSource.h
+++ include/clang/Sema/MultiplexExternalSemaSource.h
@@ -148,8 +148,10 @@
   /// \brief Print any statistics that have been gathered regarding
   /// the external AST source.
   void PrintStats() override;
-  
-  
+
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  Module *getModule(unsigned ID) override;
+
   /// \brief Perform layout on the given record.
   ///
   /// This routine allows the external AST source to provide an specific 


Index: lib/Sema/MultiplexExternalSemaSource.cpp
===
--- lib/Sema/MultiplexExternalSemaSource.cpp
+++ lib/Sema/MultiplexExternalSemaSource.cpp
@@ -164,6 +164,13 @@
 Sources[i]->PrintStats();
 }
 
+Module *MultiplexExternalSemaSource::getModule(unsigned ID) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+if (auto M = Sources[i]->getModule(ID))
+  return M;
+  return nullptr;
+}
+
 bool MultiplexExternalSemaSource::layoutRecordType(const RecordDecl *Record,
uint64_t &Size, 
uint64_t &Alignment,
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===
--- include/clang/Sema/MultiplexExternalSemaSource.h
+++ include/clang/Sema/MultiplexExternalSemaSource.h
@@ -148,8 +148,10 @@
   /// \brief Print any statistics that have been gathered regarding
   /// the external AST source.
   void PrintStats() override;
-  
-  
+
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  Module *getModule(unsigned ID) override;
+
   /// \brief Perform layout on the given record.
   ///
   /// This routine allows the external AST source to provide an specific 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread David Chisnall via Phabricator via cfe-commits

theraven added a comment.

This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` for a) 
built-in type that can hold a capability.  Having `unw_word_t` be `uintptr_t` 
made LLVM's libunwind considerably easier to port than the HP unwinder would 
have been, because `uintptr_t` is a type that is able to hold either any 
integer-register type or a pointer, whereas neither `uint32_t` nor `uint64_t` 
is.  This will be true for any architecture that supports any kind of 
fat-pointer representation.

What is the motivation for this change?  It appears to be changing working code 
in such a way that removes future proofing.

Replacing integer casts with `void*` casts and using `PRIxPTR` consistently 
would make life easier, though the use of `PRIxPTR` vs `PRIXPTR` seems 
inconsistent (as is the original use of `%x` vs `%X`.


https://reviews.llvm.org/D39365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In https://reviews.llvm.org/D39365#910590, @theraven wrote:

> This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` for 
> a) built-in type that can hold a capability.  Having `unw_word_t` be 
> `uintptr_t`

For understanding, I guess you meant "Having `unw_word_t` be `uint64_t`" here? 
Because othewise, that's exactly the change I'm doing - currently it's 
`uint64_t` while I'm proposing making it `uintptr_t` - that you're saying is 
easier to work with?

>   made LLVM's libunwind considerably easier to port than the HP unwinder 
> would have been, because `uintptr_t` is a type that is able to hold either 
> any integer-register type or a pointer, whereas neither `uint32_t` nor 
> `uint64_t` is.  This will be true for any architecture that supports any kind 
> of fat-pointer representation.  
>
> 
> What is the motivation for this change?  It appears to be changing working 
> code in such a way that removes future proofing.
> 
> Replacing integer casts with `void*` casts and using `PRIxPTR` consistently 
> would make life easier,

The original root cause that triggered me to write this, is that currently 
`unw_word_t` is `uint32_t` for ARM EHABI but `uint64_t` for everything else. 
Since I want to add support for ARM/DWARF (https://reviews.llvm.org/D39251), 
this required adding casts in UnwindLevel1.c which currently implicitly assumes 
that `unw_word_t` is `uint64_t`. Instead of adding such casts, I made one patch 
to change it to consistently be `uint64_t` (https://reviews.llvm.org/D39280) 
even on ARM EHABI, but that got the review comment that `unw_word_t` should 
match `uintptr_t`, thus I wrote this patch.

So the options currently seem to be:

- Keep the inconsistency and add ifdefs for the context size for ARM (which 
would be different for EHABI vs DWARF), where we already have ifdefs for the 
context size depending on if WMMX is enabled or not - that would make 4 
different hardcoded sizes in `__libunwind_config.h`.
- Make `unw_word_t` be `uint32_t` on ARM, `uint64_t` everywhere else (minimal 
change from status quo), add casts of one form or another in UnwindLevel1.c 
(https://reviews.llvm.org/D39251)
- Make `unw_word_t` be `uin64_t` consistently everywhere 
(https://reviews.llvm.org/D39280)
- Make `unw_word_t` be `uintptr_t` (this one)

I don't have too much of a preference, as long as people settle on one - so far 
every review has pointed me in a different direction.

> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
> original use of `%x` vs `%X`.

Yes, I've kept these as inconsistent as they were originally - if peferred I 
can make the ones I touch consistently either upper or lower case.

https://reviews.llvm.org/D39365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39419: Fix crash when parsing objective-c++ containing invalid lambda

2017-10-30 Thread Jan Korous via Phabricator via cfe-commits

jkorous-apple created this revision.

https://reviews.llvm.org/D39419

Files:
  lib/Parse/ParseExprCXX.cpp
  test/Parser/objcxx11-invalid-lambda.cpp


Index: test/Parser/objcxx11-invalid-lambda.cpp
===
--- /dev/null
+++ test/Parser/objcxx11-invalid-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -x objective-c++ -std=c++11 %s
+
+void foo() {  // expected-note {{to match this '{'}}
+  int bar;
+  auto baz = [
+  bar(  // expected-note {{to match this '('}} expected-note {{to match 
this '('}}
+foo_undeclared() // expected-error{{use of undeclared identifier 
'foo_undeclared'}} expected-error{{use of undeclared identifier 
'foo_undeclared'}}
+  /* ) */   
+] () { };   // expected-error{{expected ')'}}
+}   // expected-error{{expected ')'}} expected-error{{expected ';' 
at end of declaration}} expected-error{{expected '}'}}
\ No newline at end of file
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -991,27 +991,34 @@
 ///
 /// Returns true if it hit something unexpected.
 bool Parser::TryParseLambdaIntroducer(LambdaIntroducer &Intro) {
-  TentativeParsingAction PA(*this);
+  {
+bool SkippedInits = false;
+TentativeParsingAction PA1(*this);
 
-  bool SkippedInits = false;
-  Optional DiagID(ParseLambdaIntroducer(Intro, &SkippedInits));
+if (ParseLambdaIntroducer(Intro, &SkippedInits)) {
+  PA1.Revert();
+  return true;
+}
 
-  if (DiagID) {
-PA.Revert();
-return true;
+if (!SkippedInits) {
+  PA1.Commit();
+  return false;
+}
+
+PA1.Revert();
   }
 
-  if (SkippedInits) {
-// Parse it again, but this time parse the init-captures too.
-PA.Revert();
-Intro = LambdaIntroducer();
-DiagID = ParseLambdaIntroducer(Intro);
-assert(!DiagID && "parsing lambda-introducer failed on reparse");
+  // Try to parse it again, but this time parse the init-captures too.
+  Intro = LambdaIntroducer();
+  TentativeParsingAction PA2(*this);
+
+  if (!ParseLambdaIntroducer(Intro)) {
+PA2.Commit();
 return false;
   }
 
-  PA.Commit();
-  return false;
+  PA2.Revert();
+  return true;
 }
 
 static void


Index: test/Parser/objcxx11-invalid-lambda.cpp
===
--- /dev/null
+++ test/Parser/objcxx11-invalid-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -x objective-c++ -std=c++11 %s
+
+void foo() {  // expected-note {{to match this '{'}}
+  int bar;
+  auto baz = [
+  bar(  // expected-note {{to match this '('}} expected-note {{to match this '('}}
+foo_undeclared() // expected-error{{use of undeclared identifier 'foo_undeclared'}} expected-error{{use of undeclared identifier 'foo_undeclared'}}
+  /* ) */   
+] () { };   // expected-error{{expected ')'}}
+}   // expected-error{{expected ')'}} expected-error{{expected ';' at end of declaration}} expected-error{{expected '}'}}
\ No newline at end of file
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -991,27 +991,34 @@
 ///
 /// Returns true if it hit something unexpected.
 bool Parser::TryParseLambdaIntroducer(LambdaIntroducer &Intro) {
-  TentativeParsingAction PA(*this);
+  {
+bool SkippedInits = false;
+TentativeParsingAction PA1(*this);
 
-  bool SkippedInits = false;
-  Optional DiagID(ParseLambdaIntroducer(Intro, &SkippedInits));
+if (ParseLambdaIntroducer(Intro, &SkippedInits)) {
+  PA1.Revert();
+  return true;
+}
 
-  if (DiagID) {
-PA.Revert();
-return true;
+if (!SkippedInits) {
+  PA1.Commit();
+  return false;
+}
+
+PA1.Revert();
   }
 
-  if (SkippedInits) {
-// Parse it again, but this time parse the init-captures too.
-PA.Revert();
-Intro = LambdaIntroducer();
-DiagID = ParseLambdaIntroducer(Intro);
-assert(!DiagID && "parsing lambda-introducer failed on reparse");
+  // Try to parse it again, but this time parse the init-captures too.
+  Intro = LambdaIntroducer();
+  TentativeParsingAction PA2(*this);
+
+  if (!ParseLambdaIntroducer(Intro)) {
+PA2.Commit();
 return false;
   }
 
-  PA.Commit();
-  return false;
+  PA2.Revert();
+  return true;
 }
 
 static void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316903 - [clang-format] Format raw string literals

2017-10-30 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Mon Oct 30 07:01:50 2017
New Revision: 316903

URL: http://llvm.org/viewvc/llvm-project?rev=316903&view=rev
Log:
[clang-format] Format raw string literals

Summary:
This patch adds raw string literal formatting.

Reviewers: djasper, klimek

Reviewed By: klimek

Subscribers: klimek, mgorny

Differential Revision: https://reviews.llvm.org/D35943

Added:
cfe/trunk/lib/Format/FormatInternal.h
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/ContinuationIndenter.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/FormatTokenLexer.h
cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/lib/Format/TokenAnalyzer.cpp
cfe/trunk/lib/Format/TokenAnalyzer.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.h
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/lib/Format/UsingDeclarationsSorter.h
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/CMakeLists.txt
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=316903&r1=316902&r2=316903&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Mon Oct 30 07:01:50 2017
@@ -1327,6 +1327,41 @@ struct FormatStyle {
   /// \brief Pointer and reference alignment style.
   PointerAlignmentStyle PointerAlignment;
 
+  /// See documentation of ``RawStringFormats``.
+  struct RawStringFormat {
+/// \brief The delimiter that this raw string format matches.
+std::string Delimiter;
+/// \brief The language of this raw string.
+LanguageKind Language;
+/// \brief The style name on which this raw string format is based on.
+/// If not specified, the raw string format is based on the style that this
+/// format is based on.
+std::string BasedOnStyle;
+bool operator==(const RawStringFormat &Other) const {
+  return Delimiter == Other.Delimiter && Language == Other.Language &&
+ BasedOnStyle == Other.BasedOnStyle;
+}
+  };
+
+  /// \brief Raw string delimiters denoting that the raw string contents are
+  /// code in a particular language and can be reformatted.
+  ///
+  /// A raw string with a matching delimiter will be reformatted assuming the
+  /// specified language based on a predefined style given by 'BasedOnStyle'.
+  /// If 'BasedOnStyle' is not found, the formatting is based on llvm style.
+  ///
+  /// To configure this in the .clang-format file, use:
+  /// \code{.yaml}
+  ///   RawStringFormats:
+  /// - Delimiter: 'pb'
+  ///   Language:  TextProto
+  ///   BasedOnStyle: llvm
+  /// - Delimiter: 'proto'
+  ///   Language:  TextProto
+  ///   BasedOnStyle: google
+  /// \endcode
+  std::vector RawStringFormats;
+
   /// \brief If ``true``, clang-format will attempt to re-flow comments.
   /// \code
   ///false:
@@ -1592,6 +1627,7 @@ struct FormatStyle {
PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
PointerAlignment == R.PointerAlignment &&
+   RawStringFormats == R.RawStringFormats &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators 
&&

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=316903&r1=316902&r2=316903&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Oct 30 07:01:50 2017
@@ -14,6 +14,7 @@
 
 #include "ContinuationIndenter.h"
 #include "BreakableToken.h"
+#include "FormatInternal.h"
 #include "WhitespaceManager.h"
 #include "clang/Basic/OperatorPrecedence.h"
 #include "clang/Basic/SourceManager.h"
@@ -76,6 +77,53 @@ static bool opensProtoMessageField(const
(LessTok.Previous && LessTok.Previous->is(tok::equal;
 }
 
+// Returns the delimiter of a raw string literal, or None if TokenText is not
+// the text of a raw string literal. The d

r316906 - Keep MSVC2015 happy after r316903

2017-10-30 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Mon Oct 30 07:30:14 2017
New Revision: 316906

URL: http://llvm.org/viewvc/llvm-project?rev=316906&view=rev
Log:
Keep MSVC2015 happy after r316903

Modified:
cfe/trunk/lib/Format/Format.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=316906&r1=316905&r2=316906&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Oct 30 07:30:14 2017
@@ -1027,8 +1027,8 @@ public:
 /*LastStartColumn=*/Env.getLastStartColumn());
 for (const auto &R : Whitespaces.generateReplacements())
   if (Result.add(R))
-return {Result, 0};
-return {Result, Penalty};
+return std::make_pair(Result, 0);
+return std::make_pair(Result, Penalty);
   }
 
 private:


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

2017-10-30 Thread Barancsuk Lilla via Phabricator via cfe-commits

barancsuk updated this revision to Diff 120820.
barancsuk marked 17 inline comments as done.
barancsuk added a comment.

Address review comments, discard ambiguously redundant macro expressions


https://reviews.llvm.org/D38688

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tidy/misc/RedundantExpressionCheck.h
  docs/clang-tidy/checks/misc-redundant-expression.rst
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -15,69 +15,71 @@
 extern int bar(int x);
 extern int bat(int x, int y);
 
-int Test(int X, int Y) {
+int TestSimpleEquivalent(int X, int Y) {
   if (X - X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent [misc-redundant-expression]
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent [misc-redundant-expression]
   if (X / X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X % X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X & X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X | X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X ^ X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X < X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X <= X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X > X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X >= X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X && X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X || X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X != (((X return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X + 1 == X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 != X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 <= X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 >= X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
 
   if ((X != 1 || Y != 1) && (X != 1 || Y != 1)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both sides of operator are equivalent
   if (P.a[X - P.x] != P.a[X - P.x]) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both sides of operator are equivalent
 
   if ((int)X < (int)X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
+  if (int(X) < int(X)) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
 
   if ( + "dummy" ==

[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

2017-10-30 Thread Barancsuk Lilla via Phabricator via cfe-commits

barancsuk added inline comments.

Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:633-635
+// The function discards  (X < M1 && X <= M2), because it can always be
+// simplified to X < M, where M is the smaller one of the two macro
+// values.

aaron.ballman wrote:
> This worries me slightly for macros because those values can be overridden 
> for different compilation targets. So the diagnostic may be correct for a 
> particular configuration of the macros, but incorrect for another. Have you 
> tested this against any large code bases to see what the quality of the 
> diagnostics looks like?
Thank you for your insight! 

Now that I come to think of it, although these expressions are redundant for 
every configuration, there is no adequate simplification for each compilation 
target, because they might change their behavior if the values vary.

A good example is the following expression, that is always redundant regardless 
of the values that the macros hold.
However, the particular macro that causes the redundancy can vary from one 
compilation target to another.
```
(X < MACRO1 || X < MACRO2)
```
If MACRO1 > MACRO2, the appropriate simplification is `(X < MACRO1 )`, 
and  if MACRO1 < MACRO2, it is `(X < MACRO2)`.

Even expressions, like `(X < MACRO1 || X != MACRO2)`, that can almost always be 
simplified to `(X != MACRO2)`, change their behavior, if MACRO1 is equal to 
MACRO2  (then they are always true), and can be considered conscious 
development choices.

In conclusion, I think, it might be better now to skip these expressions, and 
check for only the ones that contain the same macro twice, like `X < MACRO && X 
> MACRO`.

https://reviews.llvm.org/D38688

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316909 - CodeGen: Fix insertion position of addrspace cast for alloca

2017-10-30 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Mon Oct 30 07:38:30 2017
New Revision: 316909

URL: http://llvm.org/viewvc/llvm-project?rev=316909&view=rev
Log:
CodeGen: Fix insertion position of addrspace cast for alloca

For non-zero alloca addr space, alloca is usually casted to default addr
space immediately.

For non-vla, alloca is inserted at AllocaInsertPt, therefore the addr
space cast should also be insterted at AllocaInsertPt. However,
for vla, alloca is inserted at the current insertion point of IRBuilder,
therefore the addr space cast should also inserted at the current
insertion point of IRBuilder.

Currently clang always insert addr space cast at AllocaInsertPt, which
causes invalid IR.

This patch fixes that.

Differential Revision: https://reviews.llvm.org/D39374

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/test/CodeGenCXX/vla.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=316909&r1=316908&r2=316909&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Oct 30 07:38:30 2017
@@ -75,7 +75,11 @@ Address CodeGenFunction::CreateTempAlloc
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
 llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
-Builder.SetInsertPoint(AllocaInsertPt);
+// When ArraySize is nullptr, alloca is inserted at AllocaInsertPt,
+// otherwise alloca is inserted at the current insertion point of the
+// builder.
+if (!ArraySize)
+  Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);

Modified: cfe/trunk/test/CodeGenCXX/vla.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vla.cpp?rev=316909&r1=316908&r2=316909&view=diff
==
--- cfe/trunk/test/CodeGenCXX/vla.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vla.cpp Mon Oct 30 07:38:30 2017
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck -check-prefixes=X64,CHECK %s
+// RUN: %clang_cc1 -std=c++11 -triple amdgcn---amdgiz %s -emit-llvm -o - | 
FileCheck -check-prefixes=AMD,CHECK %s
 
 template
 struct S {
@@ -9,7 +10,7 @@ template int S::n = 5;
 int f() {
   // Make sure that the reference here is enough to trigger the instantiation 
of
   // the static data member.
-  // CHECK: @_ZN1SIiE1nE = linkonce_odr global i32 5
+  // CHECK: @_ZN1SIiE1nE = linkonce_odr{{.*}} global i32 5
   int a[S::n];
   return sizeof a;
 }
@@ -17,10 +18,18 @@ int f() {
 // rdar://problem/9506377
 void test0(void *array, int n) {
   // CHECK-LABEL: define void @_Z5test0Pvi(
-  // CHECK:  [[ARRAY:%.*]] = alloca i8*, align 8
-  // CHECK-NEXT: [[N:%.*]] = alloca i32, align 4
-  // CHECK-NEXT: [[REF:%.*]] = alloca i16*, align 8
-  // CHECK-NEXT: [[S:%.*]] = alloca i16, align 2
+  // X64:[[ARRAY:%.*]] = alloca i8*, align 8
+  // AMD:[[ARRAY0:%.*]] = alloca i8*, align 8, addrspace(5)
+  // AMD-NEXT:   [[ARRAY:%.*]] = addrspacecast i8* addrspace(5)* [[ARRAY0]] to 
i8**
+  // X64-NEXT:   [[N:%.*]] = alloca i32, align 4
+  // AMD:[[N0:%.*]] = alloca i32, align 4, addrspace(5)
+  // AMD-NEXT:   [[N:%.*]] = addrspacecast i32 addrspace(5)* [[N0]] to i32*
+  // X64-NEXT:   [[REF:%.*]] = alloca i16*, align 8
+  // AMD:[[REF0:%.*]] = alloca i16*, align 8, addrspace(5)
+  // AMD-NEXT:   [[REF:%.*]] = addrspacecast i16* addrspace(5)* [[REF0]] to 
i16**
+  // X64-NEXT:   [[S:%.*]] = alloca i16, align 2
+  // AMD:[[S0:%.*]] = alloca i16, align 2, addrspace(5)
+  // AMD-NEXT:   [[S:%.*]] = addrspacecast i16 addrspace(5)* [[S0]] to i16*
   // CHECK-NEXT: store i8* 
   // CHECK-NEXT: store i32
 
@@ -59,6 +68,8 @@ void test0(void *array, int n) {
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
+  // AMD: %__end = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -75,13 +86,16 @@ void test2(int b) {
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* 
{{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //CHECK-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //AMD-NEXT: store i

[PATCH] D39374: CodeGen: Fix insertion position of addrspace cast for alloca

2017-10-30 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316909: CodeGen: Fix insertion position of addrspace cast 
for alloca (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D39374?vs=120609&id=120823#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39374

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/test/CodeGenCXX/vla.cpp


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -75,7 +75,11 @@
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
 llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
-Builder.SetInsertPoint(AllocaInsertPt);
+// When ArraySize is nullptr, alloca is inserted at AllocaInsertPt,
+// otherwise alloca is inserted at the current insertion point of the
+// builder.
+if (!ArraySize)
+  Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
Index: cfe/trunk/test/CodeGenCXX/vla.cpp
===
--- cfe/trunk/test/CodeGenCXX/vla.cpp
+++ cfe/trunk/test/CodeGenCXX/vla.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck -check-prefixes=X64,CHECK %s
+// RUN: %clang_cc1 -std=c++11 -triple amdgcn---amdgiz %s -emit-llvm -o - | 
FileCheck -check-prefixes=AMD,CHECK %s
 
 template
 struct S {
@@ -9,18 +10,26 @@
 int f() {
   // Make sure that the reference here is enough to trigger the instantiation 
of
   // the static data member.
-  // CHECK: @_ZN1SIiE1nE = linkonce_odr global i32 5
+  // CHECK: @_ZN1SIiE1nE = linkonce_odr{{.*}} global i32 5
   int a[S::n];
   return sizeof a;
 }
 
 // rdar://problem/9506377
 void test0(void *array, int n) {
   // CHECK-LABEL: define void @_Z5test0Pvi(
-  // CHECK:  [[ARRAY:%.*]] = alloca i8*, align 8
-  // CHECK-NEXT: [[N:%.*]] = alloca i32, align 4
-  // CHECK-NEXT: [[REF:%.*]] = alloca i16*, align 8
-  // CHECK-NEXT: [[S:%.*]] = alloca i16, align 2
+  // X64:[[ARRAY:%.*]] = alloca i8*, align 8
+  // AMD:[[ARRAY0:%.*]] = alloca i8*, align 8, addrspace(5)
+  // AMD-NEXT:   [[ARRAY:%.*]] = addrspacecast i8* addrspace(5)* [[ARRAY0]] to 
i8**
+  // X64-NEXT:   [[N:%.*]] = alloca i32, align 4
+  // AMD:[[N0:%.*]] = alloca i32, align 4, addrspace(5)
+  // AMD-NEXT:   [[N:%.*]] = addrspacecast i32 addrspace(5)* [[N0]] to i32*
+  // X64-NEXT:   [[REF:%.*]] = alloca i16*, align 8
+  // AMD:[[REF0:%.*]] = alloca i16*, align 8, addrspace(5)
+  // AMD-NEXT:   [[REF:%.*]] = addrspacecast i16* addrspace(5)* [[REF0]] to 
i16**
+  // X64-NEXT:   [[S:%.*]] = alloca i16, align 2
+  // AMD:[[S0:%.*]] = alloca i16, align 2, addrspace(5)
+  // AMD-NEXT:   [[S:%.*]] = addrspacecast i16 addrspace(5)* [[S0]] to i16*
   // CHECK-NEXT: store i8* 
   // CHECK-NEXT: store i32
 
@@ -59,6 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
+  // AMD: %__end = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -75,13 +86,16 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* 
{{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //CHECK-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
 
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
+  // AMD: %__end = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
@@ -105,7 +119,8 @@
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS:%.*]] = udiv i64 [[VLA_SIZEOF]], 
[[VLA_SIZEOF_DIM2]]
   //CHECK-NEXT: [[VLA_END_INDEX:%.*]] = mul nsw i64 [[VLA_NUM_ELEMENTS]], 
[[VLA_DIM2_PRE]]
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* 
{{%.*}}, i64 [[VLA_END_INDEX]]
-  //CHECK-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //AMD-NEXT: store i32* [

r316910 - [clang-format] Handle CRLF correctly when formatting escaped newlines

2017-10-30 Thread Krasimir Georgiev via cfe-commits

Author: krasimir
Date: Mon Oct 30 07:41:34 2017
New Revision: 316910

URL: http://llvm.org/viewvc/llvm-project?rev=316910&view=rev
Log:
[clang-format] Handle CRLF correctly when formatting escaped newlines

Subscribers: klimek

Differential Revision: https://reviews.llvm.org/D39420

Contributed by @peterbudai!

Modified:
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=316910&r1=316909&r2=316910&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Mon Oct 30 07:41:34 2017
@@ -554,13 +554,21 @@ FormatToken *FormatTokenLexer::getNextTo
   // take them into account as whitespace - this pattern is quite frequent
   // in macro definitions.
   // FIXME: Add a more explicit test.
-  while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' &&
- FormatTok->TokenText[1] == '\n') {
+  while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\') {
+unsigned SkippedWhitespace = 0;
+if (FormatTok->TokenText.size() > 2 &&
+(FormatTok->TokenText[1] == '\r' && FormatTok->TokenText[2] == '\n'))
+  SkippedWhitespace = 3;
+else if (FormatTok->TokenText[1] == '\n')
+  SkippedWhitespace = 2;
+else
+  break;
+
 ++FormatTok->NewlinesBefore;
-WhitespaceLength += 2;
-FormatTok->LastNewlineOffset = 2;
+WhitespaceLength += SkippedWhitespace;
+FormatTok->LastNewlineOffset = SkippedWhitespace;
 Column = 0;
-FormatTok->TokenText = FormatTok->TokenText.substr(2);
+FormatTok->TokenText = FormatTok->TokenText.substr(SkippedWhitespace);
   }
 
   FormatTok->WhitespaceRange = SourceRange(

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=316910&r1=316909&r2=316910&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Oct 30 07:41:34 2017
@@ -2629,14 +2629,39 @@ TEST_F(FormatTest, FormatUnbalancedStruc
 }
 
 TEST_F(FormatTest, EscapedNewlines) {
-  EXPECT_EQ(
-  "#define A \\\n  int i;  \\\n  int j;",
-  format("#define A \\\nint i;\\\n  int j;", getLLVMStyleWithColumns(11)));
+  FormatStyle Narrow = getLLVMStyleWithColumns(11);
+  EXPECT_EQ("#define A \\\n  int i;  \\\n  int j;",
+format("#define A \\\nint i;\\\n  int j;", Narrow));
   EXPECT_EQ("#define A\n\nint i;", format("#define A \\\n\n int i;"));
   EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
   EXPECT_EQ("/* \\  \\  \\\n */", format("\\\n/* \\  \\  \\\n */"));
   EXPECT_EQ("", format(""));
 
+  FormatStyle AlignLeft = getLLVMStyle();
+  AlignLeft.AlignEscapedNewlines = FormatStyle::ENAS_Left;
+  EXPECT_EQ("#define MACRO(x) \\\n"
+"private: \\\n"
+"  int x(int a);\n",
+format("#define MACRO(x) \\\n"
+   "private: \\\n"
+   "  int x(int a);\n",
+   AlignLeft));
+
+  // CRLF line endings
+  EXPECT_EQ("#define A \\\r\n  int i;  \\\r\n  int j;",
+format("#define A \\\r\nint i;\\\r\n  int j;", Narrow));
+  EXPECT_EQ("#define A\r\n\r\nint i;", format("#define A \\\r\n\r\n int i;"));
+  EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
+  EXPECT_EQ("/* \\  \\  \\\r\n */", format("\\\r\n/* \\  \\  \\\r\n */"));
+  EXPECT_EQ("", format(""));
+  EXPECT_EQ("#define MACRO(x) \\\r\n"
+"private: \\\r\n"
+"  int x(int a);\r\n",
+format("#define MACRO(x) \\\r\n"
+   "private: \\\r\n"
+   "  int x(int a);\r\n",
+   AlignLeft));
+
   FormatStyle DontAlign = getLLVMStyle();
   DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
   DontAlign.MaxEmptyLinesToKeep = 3;
@@ -2659,7 +2684,8 @@ TEST_F(FormatTest, EscapedNewlines) {
"\\\n"
"  public: \\\n"
"void baz(); \\\n"
-   "  };", DontAlign));
+   "  };",
+   DontAlign));
 }
 
 TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

> however as far as I see this will mean the LoopUnroller AST matchers can't be 
> reused unless I change them.

Hmm. What is the problem here? I'm expecting library linkage issues (i.e. 
libAnalysis is not linked to libASTMatchers directly), but i guess it'd better 
to have the whole thing in libAnalysis then (Peter, WDYT?). Or maybe we could 
move the code around a bit, so that we're still in libStaticAnalyzerCore but at 
roughly the same moment of time.

Because you're duplicating the solution for exact same problem, using different 
technology, and this problem is not trivial (eg. you still need to consider 
reference-type declstmts and initializer lists which are not taken into account 
in your solution yet) so i'd dislike the idea of duplicating it.




Comment at: lib/Analysis/CallGraph.cpp:104
+
+  void markModifiedVars(Stmt *S) {
+// Increment/Decrement, taking address of variable

A hint, even if we don't use visitors: the whole point of having a visitor is 
about not needing to make a pattern-matching (switch or sequence of dyn_casts) 
by statement kind yourself, like you do in this function. You already have 
VisitUnaryOperator, VisitBinaryOperator, VisitCallExpr, etc., so you can add 
the code there.


Repository:
  rL LLVM

https://reviews.llvm.org/D37897



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread David Chisnall via Phabricator via cfe-commits

theraven added a comment.

In https://reviews.llvm.org/D39365#910622, @mstorsjo wrote:

> In https://reviews.llvm.org/D39365#910590, @theraven wrote:
>
> > This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` 
> > for a) built-in type that can hold a capability.  Having `unw_word_t` be 
> > `uintptr_t`
>
>
> For understanding, I guess you meant "Having `unw_word_t` be `uint64_t`" 
> here? Because othewise, that's exactly the change I'm doing - currently it's 
> `uint64_t` while I'm proposing making it `uintptr_t` - that you're saying is 
> easier to work with?

Sorry - it looks as if I read the diff back to front.  I seem to be less awake 
than I thought today...

Reading the diff the correct way around, this seems like a definite improvement.

>> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
>> original use of `%x` vs `%X`.
> 
> Yes, I've kept these as inconsistent as they were originally - if peferred I 
> can make the ones I touch consistently either upper or lower case.

I'd generally prefer `PRIxPTR`, because most of the time I either don't care or 
want to copy and paste for comparison with objdump output (which uses lower 
case).

https://reviews.llvm.org/D39365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D39121#910440, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39121#909255, @aaron.ballman wrote:
>
> > My only remaining concern is with the diagnostic message/fixit interaction 
> > itself. Let's see if @alexfh has any suggestions there, or we think of an 
> > improvement ourselves.
>
>
> What do you mean by message/fixit interaction and what is your concern there?

The diagnostic tells the user that you surround the arg to strlen with parens 
to silence the diagnostic, but the fixit doesn't do that -- it moves the 
addition to the result. That's confusing behavior. However, if you add another 
fixit to surround the arg with parens (and leave in the existing one), then 
there are conflicting fixits (you don't want to apply them both) which would 
also be confusing.

https://reviews.llvm.org/D39121

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39008: [CodeGen] Propagate may-alias'ness of lvalues with TBAA info

2017-10-30 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev updated this revision to Diff 120828.
kosarev added a comment.

- Fixed comparing and hashing of TBAA access descriptors.
- Rebased on top of https://reviews.llvm.org/D39177.

Thanks John!


https://reviews.llvm.org/D39008

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGValue.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTBAA.cpp
  lib/CodeGen/CodeGenTBAA.h

Index: lib/CodeGen/CodeGenTBAA.h
===
--- lib/CodeGen/CodeGenTBAA.h
+++ lib/CodeGen/CodeGenTBAA.h
@@ -32,11 +32,22 @@
 namespace CodeGen {
 class CGRecordLayout;
 
+// TBAAAccessKind - A kind of TBAA memory access descriptor.
+enum class TBAAAccessKind : unsigned {
+  Ordinary,
+  MayAlias,
+};
+
 // TBAAAccessInfo - Describes a memory access in terms of TBAA.
 struct TBAAAccessInfo {
+  TBAAAccessInfo(TBAAAccessKind Kind, llvm::MDNode *BaseType,
+ llvm::MDNode *AccessType, uint64_t Offset)
+: Kind(Kind), BaseType(BaseType), AccessType(AccessType), Offset(Offset)
+  {}
+
   TBAAAccessInfo(llvm::MDNode *BaseType, llvm::MDNode *AccessType,
  uint64_t Offset)
-: BaseType(BaseType), AccessType(AccessType), Offset(Offset)
+: TBAAAccessInfo(TBAAAccessKind::Ordinary, BaseType, AccessType, Offset)
   {}
 
   explicit TBAAAccessInfo(llvm::MDNode *AccessType)
@@ -47,12 +58,31 @@
 : TBAAAccessInfo(/* AccessType= */ nullptr)
   {}
 
+  static TBAAAccessInfo getMayAliasInfo() {
+return TBAAAccessInfo(TBAAAccessKind::MayAlias, /* BaseType= */ nullptr,
+  /* AccessType= */ nullptr, /* Offset= */ 0);
+  }
+
+  bool isMayAlias() const { return Kind == TBAAAccessKind::MayAlias; }
+
   bool operator==(const TBAAAccessInfo &Other) const {
-return BaseType == Other.BaseType &&
+return Kind == Other.Kind &&
+   BaseType == Other.BaseType &&
AccessType == Other.AccessType &&
Offset == Other.Offset;
   }
 
+  bool operator!=(const TBAAAccessInfo &Other) const {
+return !(*this == Other);
+  }
+
+  explicit operator bool() const {
+return *this != TBAAAccessInfo();
+  }
+
+  /// Kind - The kind of the access descriptor.
+  TBAAAccessKind Kind;
+
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
@@ -139,14 +169,15 @@
   /// getAccessTagInfo - Get TBAA tag for a given memory access.
   llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
 
-  /// getMayAliasAccessInfo - Get TBAA information that represents may-alias
-  /// accesses.
-  TBAAAccessInfo getMayAliasAccessInfo();
-
   /// mergeTBAAInfoForCast - Get merged TBAA information for the purpose of
   /// type casts.
   TBAAAccessInfo mergeTBAAInfoForCast(TBAAAccessInfo SourceInfo,
   TBAAAccessInfo TargetInfo);
+
+  /// mergeTBAAInfoForConditionalOperator - Get merged TBAA information for the
+  /// purpose of conditional operator.
+  TBAAAccessInfo mergeTBAAInfoForConditionalOperator(TBAAAccessInfo InfoA,
+ TBAAAccessInfo InfoB);
 };
 
 }  // end namespace CodeGen
@@ -156,30 +187,34 @@
 
 template<> struct DenseMapInfo {
   static clang::CodeGen::TBAAAccessInfo getEmptyKey() {
+unsigned UnsignedKey = DenseMapInfo::getEmptyKey();
 return clang::CodeGen::TBAAAccessInfo(
+  static_cast(UnsignedKey),
   DenseMapInfo::getEmptyKey(),
   DenseMapInfo::getEmptyKey(),
   DenseMapInfo::getEmptyKey());
   }
 
   static clang::CodeGen::TBAAAccessInfo getTombstoneKey() {
+unsigned UnsignedKey = DenseMapInfo::getTombstoneKey();
 return clang::CodeGen::TBAAAccessInfo(
+  static_cast(UnsignedKey),
   DenseMapInfo::getTombstoneKey(),
   DenseMapInfo::getTombstoneKey(),
   DenseMapInfo::getTombstoneKey());
   }
 
   static unsigned getHashValue(const clang::CodeGen::TBAAAccessInfo &Val) {
-return DenseMapInfo::getHashValue(Val.BaseType) ^
+auto KindValue = static_cast(Val.Kind);
+return DenseMapInfo::getHashValue(KindValue) ^
+   DenseMapInfo::getHashValue(Val.BaseType) ^
DenseMapInfo::getHashValue(Val.AccessType) ^
DenseMapInfo::getHashValue(Val.Offset);
   }
 
   static bool isEqual(const clang::CodeGen::TBAAAccessInfo &LHS,
   const clang::CodeGen::TBAAAccessInfo &RHS) {
-return LHS.BaseType == RHS.BaseType &&
-   LHS.AccessType == RHS.AccessType &&
-   LHS.Offset == RHS.Offset;
+return LHS == RHS;
   }
 };
 
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -88,6 +88,25 @@
   return false;
 }
 
+///

Re: r316403 - [Analyzer] Fix for the memory leak: fix typo in if-statement.

2017-10-30 Thread David Blaikie via cfe-commits

I realize this was changed in a follow-up commit anyway, but for future
reference: There's no need (& best to avoid - simpler to read, avoids bugs
like this, etc) to conditionalize delete like this. Delete is a no-op on
null pointers anyway, so this dtor should just contain an unconditional
"delete BdyFrm;"

On Mon, Oct 23, 2017 at 6:09 PM George Karpenkov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: george.karpenkov
> Date: Mon Oct 23 18:09:43 2017
> New Revision: 316403
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316403&view=rev
> Log:
> [Analyzer] Fix for the memory leak: fix typo in if-statement.
>
> Modified:
> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
>
> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=316403&r1=316402&r2=316403&view=diff
>
> ==
> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Mon Oct 23 18:09:43 2017
> @@ -604,7 +604,7 @@ AnalysisDeclContext::~AnalysisDeclContex
>  }
>
>  AnalysisDeclContextManager::~AnalysisDeclContextManager() {
> -  if (!BdyFrm)
> +  if (BdyFrm)
>  delete BdyFrm;
>  }
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r316536 - [Analyzer] Store BodyFarm in std::unique_ptr

2017-10-30 Thread David Blaikie via cfe-commits

Could this be a value rather than indirected through a unique_ptr? Simply:

  BodyFarm BdyFrm;

& initialized in the ctors init list?

On Tue, Oct 24, 2017 at 4:53 PM George Karpenkov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: george.karpenkov
> Date: Tue Oct 24 16:53:19 2017
> New Revision: 316536
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316536&view=rev
> Log:
> [Analyzer] Store BodyFarm in std::unique_ptr
>
> Differential Revision: https://reviews.llvm.org/D39220
>
> Modified:
> cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=316536&r1=316535&r2=316536&view=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
> +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Tue Oct 24
> 16:53:19 2017
> @@ -421,7 +421,7 @@ class AnalysisDeclContextManager {
>
>/// Pointer to a factory for creating and caching implementations for
> common
>/// methods during the analysis.
> -  BodyFarm *BdyFrm = nullptr;
> +  std::unique_ptr BdyFrm;
>
>/// Flag to indicate whether or not bodies should be synthesized
>/// for well-known functions.
> @@ -438,8 +438,6 @@ public:
>   bool addCXXNewAllocator = true,
>   CodeInjector *injector = nullptr);
>
> -  ~AnalysisDeclContextManager();
> -
>AnalysisDeclContext *getContext(const Decl *D);
>
>bool getUseUnoptimizedCFG() const {
>
> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=316536&r1=316535&r2=316536&view=diff
>
> ==
> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Tue Oct 24 16:53:19 2017
> @@ -306,8 +306,8 @@ AnalysisDeclContext *AnalysisDeclContext
>
>  BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
>if (!BdyFrm)
> -BdyFrm = new BodyFarm(ASTCtx, Injector.get());
> -  return BdyFrm;
> +BdyFrm = llvm::make_unique(ASTCtx, Injector.get());
> +  return BdyFrm.get();
>  }
>
>  const StackFrameContext *
> @@ -603,11 +603,6 @@ AnalysisDeclContext::~AnalysisDeclContex
>}
>  }
>
> -AnalysisDeclContextManager::~AnalysisDeclContextManager() {
> -  if (BdyFrm)
> -delete BdyFrm;
> -}
> -
>  LocationContext::~LocationContext() {}
>
>  LocationContextManager::~LocationContextManager() {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

2017-10-30 Thread David Blaikie via cfe-commits

On Mon, Oct 23, 2017 at 5:01 PM George Karpenkov via Phabricator via
cfe-commits  wrote:

> george.karpenkov added a comment.
>
> @dcoughlin the context I was thinking about is that if everyone
> consistently runs `clang-format` (if we want that), then we never would
> have discussion.
> The alternative is that every run of `clang-format` would be followed by
> manually reverting changes which were introduced by mistake (in this case,
> because the file was moved).
>

clang-format has script (git-clang-format) for formatting only a diff, use
that or something like it so you're not reformatting unrelated lines. If
you're changing so much of the file, or it's so malformatted that local
format updates are going to be really inconsistent, reformat the entire
file in a standalone commit first, then submit your semantic changes
separately.

(also there's a way to setup clang-format to "format changed lines on-save"
in vim, which is what I use - now I basically don't think about formatting
:) )

>
>
> https://reviews.llvm.org/D39208
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

2017-10-30 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#910715, @aaron.ballman wrote:

> The diagnostic tells the user that you surround the arg to strlen with parens 
> to silence the diagnostic, but the fixit doesn't do that -- it moves the 
> addition to the result. That's confusing behavior. However, if you add 
> another fixit to surround the arg with parens (and leave in the existing 
> one), then there are conflicting fixits (you don't want to apply them both) 
> which would also be confusing.

Then, I think we should rephrase the diagnostic message again. I am confident 
that in at least 99.9% of the cases adding 1 to the argument is a mistake. So 
the primary suggestion should be to move the addition to the result instead. 
The suggestion to put the argument in extra parentheses should be secondary, 
maybe also put in parentheses.

https://reviews.llvm.org/D39121

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316914 - Fix PR#35119 : set_union misbehaves with move_iterators. Thanks to Denis Yaroshevskiy for both the bug report and the fix.

2017-10-30 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Mon Oct 30 08:50:00 2017
New Revision: 316914

URL: http://llvm.org/viewvc/llvm-project?rev=316914&view=rev
Log:
Fix PR#35119 : set_union misbehaves with move_iterators. Thanks to Denis 
Yaroshevskiy for both the bug report and the fix.

Added:

libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
Modified:
libcxx/trunk/include/algorithm

Modified: libcxx/trunk/include/algorithm
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=316914&r1=316913&r2=316914&view=diff
==
--- libcxx/trunk/include/algorithm (original)
+++ libcxx/trunk/include/algorithm Mon Oct 30 08:50:00 2017
@@ -5547,9 +5547,9 @@ __set_union(_InputIterator1 __first1, _I
 }
 else
 {
-*__result = *__first1;
 if (!__comp(*__first1, *__first2))
 ++__first2;
+*__result = *__first1;
 ++__first1;
 }
 }

Added: 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp?rev=316914&view=auto
==
--- 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 (added)
+++ 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 Mon Oct 30 08:50:00 2017
@@ -0,0 +1,44 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template
+//   requires OutputIterator
+// && OutputIterator
+// && Predicate
+// && Predicate
+//   OutIter
+//   set_union(InIter1 first1, InIter1 last1, InIter2 first2, InIter2 last2,
+// OutIter result, Compare comp);
+
+#include 
+#include 
+#include 
+#include 
+
+#include "MoveOnly.h"
+
+
+int main()
+{
+std::vector lhs, rhs;
+lhs.push_back(MoveOnly(2));
+rhs.push_back(MoveOnly(2));
+
+std::vector res;
+std::set_union(std::make_move_iterator(lhs.begin()),
+   std::make_move_iterator(lhs.end()),
+   std::make_move_iterator(rhs.begin()),
+   std::make_move_iterator(rhs.end()), 
std::back_inserter(res));
+
+assert(res.size() == 1);
+assert(res[0].get() == 2);
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39405: std::set_union accesses values after move.

2017-10-30 Thread Marshall Clow via Phabricator via cfe-commits

mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

Committed as revision 316914. Thanks!


https://reviews.llvm.org/D39405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In https://reviews.llvm.org/D39365#910713, @theraven wrote:

> Sorry - it looks as if I read the diff back to front.  I seem to be less 
> awake than I thought today...
>
> Reading the diff the correct way around, this seems like a definite 
> improvement.


Ok, thanks! Then it's just left to see if @rnk agrees with @compnerd 's 
reasoning.

>>> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
>>> original use of `%x` vs `%X`.
>> 
>> Yes, I've kept these as inconsistent as they were originally - if peferred I 
>> can make the ones I touch consistently either upper or lower case.
> 
> I'd generally prefer `PRIxPTR`, because most of the time I either don't care 
> or want to copy and paste for comparison with objdump output (which uses 
> lower case).

Ok, I'll make it consistent in the next update or when committing.


https://reviews.llvm.org/D39365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

2017-10-30 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added a comment.

> The only drawback is that the cloned function's (the thunk's) 
> DILocalVariableNodes will not appear in the newly cloned DISubprogram's node 
> variable list. Instead, the new node points to the original function's 
> variable list. However, since the only difference between the original 
> function and the thunk is the this-ptr adjustment, the resulting debug 
> information is correct, at least in the test cases I have looked at.

Does the thunk contain any local variables other than function arguments?
What I'm getting at is: If this is only a thunk, do we need any local variables 
at all?




Comment at: lib/CodeGen/CGVTables.cpp:130
+// Furthermore, the function resolves any DILocalVariable nodes referenced
+// by dbg.value intrinsics so they can be properly mapped during cloning.
+static void resolveTopLevelMetadata(llvm::Function *Fn,

`///`



Comment at: lib/CodeGen/CGVTables.cpp:140
+
+  // Find all debug.declare intrinsics and resolve the DILocalVariable nodes
+  // they are referencing.

`debug.declare` -> (llvm.)`dbg.declare`.



Comment at: lib/CodeGen/CGVTables.cpp:143
+  for (llvm::Function::iterator BB = Fn->begin(), E = Fn->end(); BB != E;
+   ++BB) {
+for (llvm::Instruction &I : *BB) {

does this work?
`for (auto &BB : Fn->getBasicBlockList())`



Comment at: lib/CodeGen/CGVTables.cpp:145
+for (llvm::Instruction &I : *BB) {
+  auto *DII = dyn_cast(&I);
+  if (DII) {

`if (auto *DII ...`


https://reviews.llvm.org/D39396



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

2017-10-30 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added inline comments.



Comment at: include/llvm/IR/Metadata.h:961
 
+  /// \brief Resolve a unique, unresolved node.
+  void resolve();

The `\brief ` is redundant and can be dropped.


https://reviews.llvm.org/D39396



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316917 - Mark test as unsupported on C++98/03, since it uses move_iterator

2017-10-30 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Mon Oct 30 09:07:59 2017
New Revision: 316917

URL: http://llvm.org/viewvc/llvm-project?rev=316917&view=rev
Log:
Mark test as unsupported on C++98/03, since it uses move_iterator

Modified:

libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp

Modified: 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp?rev=316917&r1=316916&r2=316917&view=diff
==
--- 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 Mon Oct 30 09:07:59 2017
@@ -19,6 +19,8 @@
 //   set_union(InIter1 first1, InIter1 last1, InIter2 first2, InIter2 last2,
 // OutIter result, Compare comp);
 
+// UNSUPPORTED: c++98, c++03
+
 #include 
 #include 
 #include 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.

2017-10-30 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.

An assertion failure was triggered in pr34779 by defining `strcpy` as `void 
(unsigned char *, unsigned char *)`, while normally it is `char *(char *, char 
*)`. The assertion is removed because normally we try not to crash in such 
cases, but simply refuse to model. For now i did not try to actually model the 
modified strcpy, because it requires more work (unhardcode `CharTy` in a lot of 
places around the checker), and supporting other aspects of the non-standard 
definition requires even more work (i.e. we try to bind the return value, need 
to disable this mechanism when the return type is void), and if we try to model 
other similar functions (are other string functions accepting unsigned chars as 
well?) it'd bloat quite a bit.

So //for now the analyzer would not find C string errors// in code that defines 
string functions in a non-standard manner. It simply tries to avoid crashing. 
To think - maybe we should add more defensive checks (eg. into 
`CallDescription`).


https://reviews.llvm.org/D39422

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string-with-signedness.c


Index: test/Analysis/string-with-signedness.c
===
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+  strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
   if (!ER)
 return state;
 
-  assert(ER->getValueType() == C.getASTContext().CharTy &&
-"CheckLocation should only be called with char* ElementRegions");
+  if (ER->getValueType() != C.getASTContext().CharTy)
+return state;
 
   // Get the size of the array.
   const SubRegion *superReg = cast(ER->getSuperRegion());
@@ -874,6 +874,8 @@
   if (!ER)
 return true; // cf top comment.
 
+  // FIXME: Does this crash when a non-standard definition
+  // of a library function is encountered?
   assert(ER->getValueType() == C.getASTContext().CharTy &&
  "IsFirstBufInBound should only be called with char* ElementRegions");
 


Index: test/Analysis/string-with-signedness.c
===
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration -analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+  strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
   if (!ER)
 return state;
 
-  assert(ER->getValueType() == C.getASTContext().CharTy &&
-"CheckLocation should only be called with char* ElementRegions");
+  if (ER->getValueType() != C.getASTContext().CharTy)
+return state;
 
   // Get the size of the array.
   const SubRegion *superReg = cast(ER->getSuperRegion());
@@ -874,6 +874,8 @@
   if (!ER)
 return true; // cf top comment.
 
+  // FIXME: Does this crash when a non-standard definition
+  // of a library function is encountered?
   assert(ER->getValueType() == C.getASTContext().CharTy &&
  "IsFirstBufInBound should only be called with char* ElementRegions");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39423: [analyzer] Left shifting a negative value is undefined

2017-10-30 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun created this revision.
Herald added subscribers: szepet, baloghadamsoftware, whisperity.

The analyzer did not return an UndefVal in case a negative value was left 
shifted.
I also added altered the UndefResultChecker to emit a clear warning in this 
case.


Repository:
  rL LLVM

https://reviews.llvm.org/D39423

Files:
  lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  test/Analysis/bitwise-ops.c


Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())


Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is undefined because the left operand is negative}}
+  }
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-30 Thread Carlos Alberto Enciso via Phabricator via cfe-commits

CarlosAlbertoEnciso added a reviewer: tberghammer.
CarlosAlbertoEnciso added a comment.

Hi Tamas,

I have added you to this review, as I think I have found something odd with the 
LLDB.

Basically after this intended patch to the compiler (to emit scoped information 
only for scoped enums) the following test fail:

tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template/TestTemplateArgs.py

I have saved the objects generated before and after the change for both 
32/64bits mode.  The test case is checking for scoped information which is 
present in the debug information.

  def test_enum_args(self):
  frame = self.prepareProcess()
  
  # Make sure "member" can be displayed and also used in an expression
  # correctly
  member = frame.FindVariable('member')
  self.assertTrue(
  member.IsValid(),
  'make sure we find a local variabble named "member"')
  
  self.assertTrue(member.GetType().GetName() ==
  'EnumTemplate')

After some debugging, 'TestTemplateArgs.py' fails when retrieving the member 
type name, which is expected to have the 'EnumTemplate'. As 
per my previous comment, that string is present; but the expression 
'member.GetType().GetName()' returns just 'EnumTemplate' causing the 
test to fail.

I would appreciate it very much your feedback.

Thanks


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D39121#910735, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39121#910715, @aaron.ballman wrote:
>
> > The diagnostic tells the user that you surround the arg to strlen with 
> > parens to silence the diagnostic, but the fixit doesn't do that -- it moves 
> > the addition to the result. That's confusing behavior. However, if you add 
> > another fixit to surround the arg with parens (and leave in the existing 
> > one), then there are conflicting fixits (you don't want to apply them both) 
> > which would also be confusing.
>
>
> Then, I think we should rephrase the diagnostic message again. I am confident 
> that in at least 99.9% of the cases adding 1 to the argument is a mistake. So 
> the primary suggestion should be to move the addition to the result instead. 
> The suggestion to put the argument in extra parentheses should be secondary, 
> maybe also put in parentheses.

Agreed -- we want to keep the fixit with moving the +1 out to the result. 
However, I'm still worried that there'd be no way for the user to know how to 
silence the warning if the code is already correct -- I don't want users to 
disable a useful check because they don't have guidance on how to silence the 
diagnostic.

Do you have concrete numbers of how many diagnostics are triggered on large 
code bases, and the false positive rate?

https://reviews.llvm.org/D39121

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.

2017-10-30 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

This patch looks good to me.
I am wondering whether it would make sense to have some kind of warning (even 
in the frontend if not in the analyzer) for using standard functions with 
non-standard signatures, so users start to file bugs for the maintainers of 
those libraries.


https://reviews.llvm.org/D39422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM

FYI: it's been difficult to perform this review because all of these reviews 
are touching the same chunk of code for something that's not been committed 
yet. It would be easier to review if all of these reviews were combined into 
the review adding the check.


https://reviews.llvm.org/D39370



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24886: Add [[clang::suppress(rule, ...)]] attribute

2017-10-30 Thread Aaron Ballman via cfe-commits

On Sun, Oct 29, 2017 at 4:42 AM, Hristo Hristov via Phabricator
 wrote:
> roccoor added a comment.
>
> Microsoft supports:
>
>   [[gsl::suppress(bounds.3)]]
>
> Clang requires:
>
>   [[gsl::suppress("bounds.3")]]
>
> Why is this inconsistency?

I believe that when we added this attribute, MSVC did not support the
attribute at all and so we did what was recommended by the C++ Core
Guideline maintainers.

It would be possible for us to support the Microsoft syntax, but it
may require special parsing support.

~Aaron

>
> Here is an example from  CppCoreGuidelines
>
>   [[suppress(bounds)]] char* raw_find(char* p, int n, char x)// find x in 
> p[0]..p[n - 1]
>   {
>   // ...
>   }
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24886
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:633-635
+// The function discards  (X < M1 && X <= M2), because it can always be
+// simplified to X < M, where M is the smaller one of the two macro
+// values.

barancsuk wrote:
> aaron.ballman wrote:
> > This worries me slightly for macros because those values can be overridden 
> > for different compilation targets. So the diagnostic may be correct for a 
> > particular configuration of the macros, but incorrect for another. Have you 
> > tested this against any large code bases to see what the quality of the 
> > diagnostics looks like?
> Thank you for your insight! 
> 
> Now that I come to think of it, although these expressions are redundant for 
> every configuration, there is no adequate simplification for each compilation 
> target, because they might change their behavior if the values vary.
> 
> A good example is the following expression, that is always redundant 
> regardless of the values that the macros hold.
> However, the particular macro that causes the redundancy can vary from one 
> compilation target to another.
> ```
> (X < MACRO1 || X < MACRO2)
> ```
> If MACRO1 > MACRO2, the appropriate simplification is `(X < MACRO1 )`, 
> and  if MACRO1 < MACRO2, it is `(X < MACRO2)`.
> 
> Even expressions, like `(X < MACRO1 || X != MACRO2)`, that can almost always 
> be simplified to `(X != MACRO2)`, change their behavior, if MACRO1 is equal 
> to MACRO2  (then they are always true), and can be considered conscious 
> development choices.
> 
> In conclusion, I think, it might be better now to skip these expressions, and 
> check for only the ones that contain the same macro twice, like `X < MACRO && 
> X > MACRO`.
> 
> In conclusion, I think, it might be better now to skip these expressions, and 
> check for only the ones that contain the same macro twice, like X < MACRO && 
> X > MACRO.

I think that approach makes the most sense, at least initially.


https://reviews.llvm.org/D38688



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-30 Thread Tamas Berghammer via Phabricator via cfe-commits

tberghammer added a comment.

2 fairly random ideas without looking into it too much:

- Can you do a diff of the debug_info dump before and after your change? 
Understanding what have changed should give us a pretty good clue about the 
issue.
- My first guess is that after your change we emit DW_TAG_enumeration_type for 
scoped enums (what seems to be the correct thing to do) instead of something 
else (not sure what) and you have to update 
https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp#L1512
 to handle it correctly.


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D39367



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

2017-10-30 Thread Balogh , Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39370#910791, @aaron.ballman wrote:

> LGTM
>
> FYI: it's been difficult to perform this review because all of these reviews 
> are touching the same chunk of code for something that's not been committed 
> yet. It would be easier to review if all of these reviews were combined into 
> the review adding the check.

I am also working for the Static Analyzer where I received the comment that 
development should be incremental to avoid huge patches. So I tried to use the 
same approach here as well.

https://reviews.llvm.org/D39370

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

2017-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D39370#910803, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39370#910791, @aaron.ballman wrote:
>
> > LGTM
> >
> > FYI: it's been difficult to perform this review because all of these 
> > reviews are touching the same chunk of code for something that's not been 
> > committed yet. It would be easier to review if all of these reviews were 
> > combined into the review adding the check.
>
>
> I am also working for the Static Analyzer where I received the comment that 
> development should be incremental to avoid huge patches. So I tried to use 
> the same approach here as well.

Incremental is definitely the way to go, but that's usually to prevent massive 
code dumps of large-scale functionality. For a single, relatively small check 
like this, I think it's fine to add all of this into one review because it's 
all so tightly related.

https://reviews.llvm.org/D39370

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread Reid Kleckner via Phabricator via cfe-commits

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D39365#909912, @compnerd wrote:

> @rnk given that the remote unwinder is completely unimplemented ATM, I think 
> that this isn't a big concern.  I'm not sure that this makes anything worse, 
> but I do feel that it is making things better (especially since 
> `RemoteAddressSpace` is currently mostly just a '// FIXME: implement').

Sounds good to me, just wanted to check. :)

https://reviews.llvm.org/D39365

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Marshall Clow via Phabricator via cfe-commits

mclow.lists added a comment.

I dislike this change fairly strongly.
I would much rather pursue a clang-based solution (since clang is being 
unhelpful here)
Don't know if we can get one, though.


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316924 - [analyzer] Left shifting a negative value is undefined

2017-10-30 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Mon Oct 30 10:06:42 2017
New Revision: 316924

URL: http://llvm.org/viewvc/llvm-project?rev=316924&view=rev
Log:
[analyzer] Left shifting a negative value is undefined

The analyzer did not return an UndefVal in case a negative value was left
shifted. I also altered the UndefResultChecker to emit a clear warning in this
case.

Differential Revision: https://reviews.llvm.org/D39423

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
cfe/trunk/test/Analysis/bitwise-ops.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp Mon Oct 30 
10:06:42 2017
@@ -137,6 +137,10 @@ void UndefResultChecker::checkPostStmt(c
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp Mon Oct 30 10:06:42 
2017
@@ -225,6 +225,8 @@ BasicValueFactory::evalAPSInt(BinaryOper
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;

Modified: cfe/trunk/test/Analysis/bitwise-ops.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bitwise-ops.c?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/test/Analysis/bitwise-ops.c (original)
+++ cfe/trunk/test/Analysis/bitwise-ops.c Mon Oct 30 10:06:42 2017
@@ -44,3 +44,10 @@ int testNegativeShift(int a) {
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39423: [analyzer] Left shifting a negative value is undefined

2017-10-30 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316924: [analyzer] Left shifting a negative value is 
undefined (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D39423?vs=120837&id=120841#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39423

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  cfe/trunk/test/Analysis/bitwise-ops.c


Index: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: cfe/trunk/test/Analysis/bitwise-ops.c
===
--- cfe/trunk/test/Analysis/bitwise-ops.c
+++ cfe/trunk/test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}


Index: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: cfe/trunk/test/Analysis/bitwise-ops.c
===
--- cfe/trunk/test/Analysis/bitwise-ops.c
+++ cfe/trunk/test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is undefined because the left operand is negative}}
+  }
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38844: [analyzer] Make issue hash related tests more concise

2017-10-30 Thread Anna Zaks via Phabricator via cfe-commits

zaks.anna added a comment.

I'd also include some info on how it's now possible to dump the issue hash. You 
introduce a new debugging function here "clang_analyzer_hashDump" but it's not 
mentioned in the commit message.

Thanks!




Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:68
   // (globals should not be invalidated, etc), hence the use of evalCall.
-  FnCheck Handler = llvm::StringSwitch(C.getCalleeName(CE))
-.Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
-.Case("clang_analyzer_checkInlined",
-  &ExprInspectionChecker::analyzerCheckInlined)
-.Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
-.Case("clang_analyzer_warnIfReached",
-  &ExprInspectionChecker::analyzerWarnIfReached)
-.Case("clang_analyzer_warnOnDeadSymbol",
-  &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
-.StartsWith("clang_analyzer_explain", 
&ExprInspectionChecker::analyzerExplain)
-.StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
-.Case("clang_analyzer_getExtent", 
&ExprInspectionChecker::analyzerGetExtent)
-.Case("clang_analyzer_printState",
-  &ExprInspectionChecker::analyzerPrintState)
-.Case("clang_analyzer_numTimesReached",
-  &ExprInspectionChecker::analyzerNumTimesReached)
-.Default(nullptr);
+  FnCheck Handler =
+  llvm::StringSwitch(C.getCalleeName(CE))

xazax.hun wrote:
> zaks.anna wrote:
> > Unrelated change?
> Since I touched this snippet I reformatted it using clang-format. Apart from 
> adding a new case before the default all other changes are formatting 
> changes. I will revert the formatting changes. So in general, we prefer to 
> minimize the diffs over converging to be clang-formatted?
It's much easier to review when the diff does not contain formatting changes 
intermixed with functional changes. Looks like you can configure clang-format 
to only format the diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D38844



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a subscriber: thakis.
lebedev.ri added a comment.

That is my diagnostic, so i guess this is the time to reply :)

In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:

> I dislike this change fairly strongly.
>  I would much rather pursue a clang-based solution (since clang is being 
> unhelpful here)
>  Don't know if we can get one, though.

As i see it, there are several options:

1. Disable that warning in `libc++` cmakelists. Be careful though, i think it 
might disable the entire 'tautological comparison' diagnostic family.
2. Use preprocessor pragmas to disable the diagnostic for the selected code, 
https://reviews.llvm.org/rL315882. Brittle, ugly, effective, has the least 
impact. <- Best?
3. In `-Wtautological-constant-compare`, ignore any comparisons that compare 
with `std::numeric_limits`. Not a fan of this solution. <- Worst?
4. Disable that diagnostic by default in clang. Also, not really a fan of this 
solution. I implemented it because it would have caught a real bug in my code 
in rather shorter time, see mail 
.
5. The essential problem, i *think* is much like the problem with unreachable 
code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
. The check does not know/care whether 
the comparison is tautological only with the current Fundamental type sizes, or 
always. Perhaps this is the proper heading?
6. ???

https://reviews.llvm.org/D39149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35780: Introduce -nostdlib++ flag to disable linking the C++ standard library.

2017-10-30 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Sounds like this ended up being useful for Android too: 
https://github.com/android-ndk/ndk/issues/105#issuecomment-324179958


https://reviews.llvm.org/D35780



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Marshall Clow via Phabricator via cfe-commits

mclow.lists added a comment.

If we have to go down this road, I'd prefer the approach used in 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?r1=315874&r2=315873&pathrev=315874


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai planned changes to this revision.
smeenai added a comment.

In https://reviews.llvm.org/D39149#910858, @mclow.lists wrote:

> If we have to go down this road, I'd prefer the approach used in 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?r1=315874&r2=315873&pathrev=315874
>
> (which is your solution #2)


All right, I'll change this patch accordingly.


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:

> I dislike this change fairly strongly.

Agreed. Would you be happier with just disabling the diagnostics around the 
problematic parts?

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> 1. Disable that warning in `libc++` cmakelists. Be careful though, i think it 
> might disable the entire 'tautological comparison' diagnostic family.

Note that one of the tautological comparisons is in a header, so it affects all 
libc++ clients and not just libc++ itself.

> 2. Use preprocessor pragmas to disable the diagnostic for the selected code, 
> https://reviews.llvm.org/rL315882. Brittle, ugly, effective, has the least 
> impact. <- Best?

I was considering doing this instead. It also seems ugly in its own way, but 
possibly less ugly than this patch.

> 5. The essential problem, i *think* is much like the problem with unreachable 
> code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
> . The check does not know/care whether 
> the comparison is tautological only with the current Fundamental type sizes, 
> or always. Perhaps this is the proper heading?

Yeah, exactly. If the warning could only fire when the comparison was *always* 
going to be tautological, that would avoid any issues like this, but I'm not 
sure how best to go about that.

https://reviews.llvm.org/D39149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Brian Cain via Phabricator via cfe-commits

bcain added a comment.

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> That is my diagnostic, so i guess this is the time to reply :)

...

> 3. In `-Wtautological-constant-compare`, ignore any comparisons that compare 
> with `std::numeric_limits`. Not a fan of this solution. <- Worst?

...

Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
could understand it if you had reservations about ignoring comparisons against 
{CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is well-qualified and 
everything.

Can you help me understand why you don't prefer it?

Also, is there any way that the warning could somehow be smart enough to know 
what sizeof(int) and sizeof(long) are?

As an aside, how widely has this new clang behavior been tested?  Is it 
possible that this new warning behavior might get rolled back if a 
big/important enough codebase triggers it?

https://reviews.llvm.org/D39149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39008: [CodeGen] Propagate may-alias'ness of lvalues with TBAA info

2017-10-30 Thread John McCall via Phabricator via cfe-commits

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D39008



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39405: std::set_union accesses values after move.

2017-10-30 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

We should backport this, right @mclow.lists?


https://reviews.llvm.org/D39405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316935 - Add a test to make sure that -Wdeprecated doesn't warn on use of 'throw()' in system headers (deprecated in C++17).

2017-10-30 Thread Richard Smith via cfe-commits

Author: rsmith
Date: Mon Oct 30 11:05:10 2017
New Revision: 316935

URL: http://llvm.org/viewvc/llvm-project?rev=316935&view=rev
Log:
Add a test to make sure that -Wdeprecated doesn't warn on use of 'throw()' in 
system headers (deprecated in C++17).

Modified:
cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
cfe/trunk/test/SemaCXX/deprecated.cpp

Modified: cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp?rev=316935&r1=316934&r2=316935&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Mon Oct 30 11:05:10 
2017
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions %s
-// RUN: %clang_cc1 -std=c++1z -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
+// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-c++1z-compat-mangling -DNO_COMPAT_MANGLING %s
-// RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
+// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
 
 #if __cplusplus > 201402L
 

Modified: cfe/trunk/test/SemaCXX/deprecated.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/deprecated.cpp?rev=316935&r1=316934&r2=316935&view=diff
==
--- cfe/trunk/test/SemaCXX/deprecated.cpp (original)
+++ cfe/trunk/test/SemaCXX/deprecated.cpp Mon Oct 30 11:05:10 2017
@@ -93,3 +93,6 @@ namespace DeprecatedCopy {
   void g() { c1 = c2; } // expected-note {{implicit copy assignment operator 
for 'DeprecatedCopy::Dtor' first required here}}
 }
 #endif
+
+# 1 "/usr/include/system-header.h" 1 3
+void system_header_function(void) throw();


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316936 - Undo accidental language mode change in this test.

2017-10-30 Thread Richard Smith via cfe-commits

Author: rsmith
Date: Mon Oct 30 11:06:18 2017
New Revision: 316936

URL: http://llvm.org/viewvc/llvm-project?rev=316936&view=rev
Log:
Undo accidental language mode change in this test.

Modified:
cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp

Modified: cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp?rev=316936&r1=316935&r2=316936&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Mon Oct 30 11:06:18 
2017
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions %s
 // RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-c++1z-compat-mangling -DNO_COMPAT_MANGLING %s
-// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
+// RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
 
 #if __cplusplus > 201402L
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

2017-10-30 Thread George Karpenkov via cfe-commits



> On Oct 30, 2017, at 8:40 AM, David Blaikie  wrote:
> 
> 
> 
> On Mon, Oct 23, 2017 at 5:01 PM George Karpenkov via Phabricator via 
> cfe-commits mailto:cfe-commits@lists.llvm.org>> 
> wrote:
> george.karpenkov added a comment.
> 
> @dcoughlin the context I was thinking about is that if everyone consistently 
> runs `clang-format` (if we want that), then we never would have discussion.
> The alternative is that every run of `clang-format` would be followed by 
> manually reverting changes which were introduced by mistake (in this case, 
> because the file was moved).
> 
> clang-format has script (git-clang-format) for formatting only a diff, use 
> that or something like it so you're not reformatting unrelated lines. If 
> you're changing so much of the file, or it's so malformatted that local 
> format updates are going to be really inconsistent, reformat the entire file 
> in a standalone commit first, then submit your semantic changes separately.

But that’s what I was using: trouble is, the file got renamed, and clang-format 
then decided to reformat all of it.
(and another question is whether it is a reasonable thing to do)

> 
> (also there's a way to setup clang-format to "format changed lines on-save" 
> in vim, which is what I use - now I basically don't think about formatting :) 
> )
>  
> 
> 
> https://reviews.llvm.org/D39208 
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/D39149#910891, @bcain wrote:

> In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
>
> > That is my diagnostic, so i guess this is the time to reply :)
>
>
> ...
>
> > 3. In `-Wtautological-constant-compare`, ignore any comparisons that 
> > compare with `std::numeric_limits`. Not a fan of this solution. <- Worst?
>
> ...
>
> Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
> could understand it if you had reservations about ignoring comparisons 
> against {CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is 
> well-qualified and everything.
>
> Can you help me understand why you don't prefer it?

That my initial bug was not caught by gcc either, even though `-Wtype-limits` 
was enabled.
Because for whatever reason gcc basically does not issue diagnostics for 
templated functions.
Even though the tautologically-compared variable was *not* dependent on the 
template parameters in any way.
See for yourself, i think this is *really* bad: https://godbolt.org/g/zkq3UD

So all-size-fits-all things like that are just asking to be done wrong.
I fail to see how `ignore any comparisons that compare with 
std::numeric_limits` would not be the same pitfall, unfortunately.

> Also, is there any way that the warning could somehow be smart enough to know 
> what sizeof(int) and sizeof(long) are?

I'm not sure i follow. It sure does know that, else it would not possible to 
diagnose anything.
Or were you talking about

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> That is my diagnostic, so i guess this is the time to reply :)
>  As i see it, there are several options:
>  ...
>
> 5. The essential problem, i *think* is much like the problem with unreachable 
> code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
> . The check does not know/care whether 
> the comparison is tautological only with the current Fundamental type sizes, 
> or always. Perhaps this is the proper heading?

?

> As an aside, how widely has this new clang behavior been tested?  Is it 
> possible that this new warning behavior might get rolled back if a 
> big/important enough codebase triggers it?

Are you talking about false-positives* or about real true-positive warnings?
So far, to the best of my knowledge, there were no reports of false-positives*.
If a false-positive is reported, and it is not obvious how to fix it, i suppose 
it might be reverted.
True-positives are obviously not the justification for the revert, but a 
validation of the validity of the diagnostic.
The cases like this one are troublesome. **If** it is possible to reduce a case 
that obviously should not warn, **then** the diagnostic should be adjusted not 
to warn.

- The fact that under *different* circumstances the comparison is not 
tautological is not a false-positive.

https://reviews.llvm.org/D39149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39284: [c++2a] Decomposed _condition_

2017-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: include/clang/Sema/DeclSpec.h:2015
 case ConditionContext:
+  return Lang.CPlusPlus2a;
 case MemberContext:

If there's no grammar ambiguities here (and I don't think there are), please 
accept this unconditionally, with an `ExtWarn`.



Comment at: lib/Parse/ParseExprCXX.cpp:1711
 /// [C++11] type-specifier-seq declarator braced-init-list
+/// [C++2a] type-specifier-seq ref-qualifier[opt] '[' identifier-list ']'
+/// brace-or-equal-initializer

This shouldn't be listed as `C++2a` until it's actually voted into the working 
draft. Listing it as `[Clang]` for a clang extension is fine.



Comment at: lib/Sema/SemaDeclCXX.cpp:696-698
+  // a for-range-declaration, or a condition in C++2a, but we parse it in more
+  // cases than that.
+  if (!D.mayHaveDecompositionDeclarator(getLangOpts())) {

Again, please don't guess what's going to be in C++2a. I would actually expect 
that we'll vote this in as a DR against C++17. It seems like a wording 
oversight to me.


https://reviews.llvm.org/D39284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

I'm thinking you could account for all possible type sizes in the existing 
(enabled by default) warning, and have a different warning for possibly 
tautological comparisons. E.g. if a `long` is being compared against `INT_MAX`, 
you know that's only tautological on some platforms, so it should go under 
`-Wpossible-tautological-constant-compare` (which would only be enabled by 
`-Weverything` and not `-Wall` or `-Wextra`); `-Wtautological-constant-compare` 
should be reserved for definitely tautological cases.


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39332: [clang-refactor] Introduce "local-qualified-rename" action.

2017-10-30 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added inline comments.



Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+  std::string NewQualifiedName) {
+  return QualifiedRenameRule(std::move(OldQualifiedName),
+ std::move(NewQualifiedName));

It might be better to find the declaration (and report error if needed) during 
in initiation, and then pass the `ND` to the class. Maybe then both 
`RenameOccurrences` and `QualifiedRenameRule` could subclass from one base 
class that actually does just this:

``` 
auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
  assert(!USRs.empty());
  return tooling::createRenameAtomicChanges(
  USRs, NewQualifiedName, Context.getASTContext().getTranslationUnitDecl());
```


https://reviews.llvm.org/D39332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:

> - The fact that under *different* circumstances the comparison is not 
> tautological is not a false-positive.

It's not technically a false positive, but my suspicion is that it'll make the 
warning a lot less useful in a lot of cases.

https://reviews.llvm.org/D39149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316939 - Fix PR35078 - recursive directory iterator's increment method throws incorrectly.

2017-10-30 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Mon Oct 30 11:43:21 2017
New Revision: 316939

URL: http://llvm.org/viewvc/llvm-project?rev=316939&view=rev
Log:
Fix PR35078 - recursive directory iterator's increment method throws 
incorrectly.

The guts of the increment method for recursive_directory_iterator
was failing to pass an error code object to calls to status/symlink_status,
which can throw under certain conditions.

This patch fixes the issues by correctly propagating the error codes.
However the noexcept still needs to be removed from the signature, as
mentioned in LWG 3014, but that change will be made in a separate commit.

Modified:
libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp

Modified: libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp?rev=316939&r1=316938&r2=316939&view=diff
==
--- libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp Mon Oct 30 
11:43:21 2017
@@ -296,24 +296,43 @@ void recursive_directory_iterator::__adv
 }
 
 bool recursive_directory_iterator::__try_recursion(error_code *ec) {
-
 bool rec_sym =
 bool(options() & directory_options::follow_directory_symlink);
+
 auto& curr_it = __imp_->__stack_.top();
 
-if (is_directory(curr_it.__entry_.status()) &&
-(!is_symlink(curr_it.__entry_.symlink_status()) || rec_sym))
-{
-std::error_code m_ec;
+bool skip_rec = false;
+std::error_code m_ec;
+if (!rec_sym) {
+  file_status st = curr_it.__entry_.symlink_status(m_ec);
+  if (m_ec && status_known(st))
+m_ec.clear();
+  if (m_ec || is_symlink(st) || !is_directory(st))
+skip_rec = true;
+} else {
+  file_status st = curr_it.__entry_.status(m_ec);
+  if (m_ec && status_known(st))
+m_ec.clear();
+  if (m_ec || !is_directory(st))
+skip_rec = true;
+}
+
+if (!skip_rec) {
 __dir_stream new_it(curr_it.__entry_.path(), __imp_->__options_, m_ec);
 if (new_it.good()) {
 __imp_->__stack_.push(_VSTD::move(new_it));
 return true;
 }
-if (m_ec) {
-__imp_.reset();
-set_or_throw(m_ec, ec,
-   "recursive_directory_iterator::operator++()");
+}
+if (m_ec) {
+const bool allow_eacess = bool(__imp_->__options_
+& directory_options::skip_permission_denied);
+if (m_ec.value() == EACCES && allow_eacess) {
+  if (ec) ec->clear();
+} else {
+  __imp_.reset();
+  set_or_throw(m_ec, ec,
+   "recursive_directory_iterator::operator++()");
 }
 }
 return false;

Modified: 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp?rev=316939&r1=316938&r2=316939&view=diff
==
--- 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
 Mon Oct 30 11:43:21 2017
@@ -237,4 +237,250 @@ TEST_CASE(access_denied_on_recursion_tes
 }
 }
 
+// See llvm.org/PR35078
+TEST_CASE(test_PR35078)
+{
+  using namespace std::experimental::filesystem;
+scoped_test_env env;
+const path testFiles[] = {
+env.create_dir("dir1"),
+env.create_dir("dir1/dir2"),
+env.create_dir("dir1/dir2/dir3"),
+env.create_file("dir1/file1"),
+env.create_file("dir1/dir2/dir3/file2")
+};
+const path startDir = testFiles[0];
+const path permDeniedDir = testFiles[1];
+const path nestedDir = testFiles[2];
+const path nestedFile = testFiles[3];
+
+// Change the permissions so we can no longer iterate
+permissions(permDeniedDir,
+perms::remove_perms|perms::group_exec
+   |perms::owner_exec|perms::others_exec);
+
+const std::error_code eacess_ec =
+std::make_error_code(std::errc::permission_denied);
+std::error_code ec = GetTestEC();
+
+const recursive_directory_iterator endIt;
+
+auto SetupState = [&](bool AllowEAccess, bool& SeenFile3) {
+  SeenFile3 = false;
+  auto Opts = AllowEAccess ? directory_options::skip_permission_denied
+  : directory_options::none;
+  recursive_directory_iterator it(startDir, Opts, ec);
+  while (!ec && it != endIt && *it != nestedDir) {
+if (*it ==

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

2017-10-30 Thread George Karpenkov via Phabricator via cfe-commits

george.karpenkov created this revision.
Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, 
aemerson.

I've also removed copy constructor to help clients not to shoot themselves in 
the foot with `BodyFarm stored = Manager.getBodyFarm()`.


https://reviews.llvm.org/D39428

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/BodyFarm.h
  lib/Analysis/AnalysisDeclContext.cpp


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(MD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(MD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -304,10 +305,8 @@
   return AC.get();
 }
 
-BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
-  if (!FunctionBodyFarm)
-FunctionBodyFarm = llvm::make_unique(ASTCtx, Injector.get());
-  return FunctionBodyFarm.get();
+BodyFarm& AnalysisDeclContextManager::getBodyFarm() {
+  return FunctionBodyFarm;
 }
 
 const StackFrameContext *
Index: include/clang/Analysis/BodyFarm.h
===
--- include/clang/Analysis/BodyFarm.h
+++ include/clang/Analysis/BodyFarm.h
@@ -39,6 +39,9 @@
   /// Factory method for creating bodies for Objective-C properties.
   Stmt *getBody(const ObjCMethodDecl *D);
 
+  /// Remove copy constructor to avoid accidental copying.
+  BodyFarm(const BodyFarm& other) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -419,9 +419,9 @@
   /// declarations from external source.
   std::unique_ptr Injector;
 
-  /// Pointer to a factory for creating and caching implementations for common
+  /// A factory for creating and caching implementations for common
   /// methods during the analysis.
-  std::unique_ptr FunctionBodyFarm;
+  BodyFarm FunctionBodyFarm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -476,7 +476,7 @@
   }
 
   /// Get and lazily create a {@code BodyFarm} instance.
-  BodyFarm *getBodyFarm();
+  BodyFarm& getBodyFarm();
 
   /// Discard all previously created AnalysisDeclContexts.
   void clear();


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Mana

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

2017-10-30 Thread David Blaikie via Phabricator via cfe-commits

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - thanks!


https://reviews.llvm.org/D39428



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Brian Cain via Phabricator via cfe-commits

bcain added a comment.

In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:

> In https://reviews.llvm.org/D39149#910891, @bcain wrote:
>
> > In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
> >
> > > That is my diagnostic, so i guess this is the time to reply :)
> >
> >
> > ...
> >
> > > 3. In `-Wtautological-constant-compare`, ignore any comparisons that 
> > > compare with `std::numeric_limits`. Not a fan of this solution. <- Worst?
> >
> > ...
> >
> > Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
> > could understand it if you had reservations about ignoring comparisons 
> > against {CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is 
> > well-qualified and everything.
> >
> > Can you help me understand why you don't prefer it?
>
>
> That my initial bug was not caught by gcc either, even though `-Wtype-limits` 
> was enabled.


...

Ok, yes, that makes sense.

>> Also, is there any way that the warning could somehow be smart enough to 
>> know what sizeof(int) and sizeof(long) are?
> 
> I'm not sure i follow. It sure does know that, else it would not possible to 
> diagnose anything.

Yes, sorry, I had the logic reversed in my head.

...

>> As an aside, how widely has this new clang behavior been tested?  Is it 
>> possible that this new warning behavior might get rolled back if a 
>> big/important enough codebase triggers it?
> 
> Are you talking about false-positives* or about real true-positive warnings?
>  So far, to the best of my knowledge, there were no reports of 
> false-positives*.
>  If a false-positive is reported, and it is not obvious how to fix it, i 
> suppose it might be reverted.
>  True-positives are obviously not the justification for the revert, but a 
> validation of the validity of the diagnostic.
>  The cases like this one are troublesome. **If** it is possible to reduce a 
> case that obviously should not warn, **then** the diagnostic should be 
> adjusted not to warn.
> 
> - The fact that under *different* circumstances the comparison is not 
> tautological is not a false-positive.

Isn't this a case that it obviously should not warn?

What about another approach: (1) could we consider Shoaib's suggestion to keep 
the exhaustive check with this name and create a new one that's limited (and 
can take the current one's place in the enabled-by-Wall) or (2) could we create 
range check functions that have the warning disabled?

Option #2 is limited to benefiting only llvm or libcxx code.  But if we narrow 
our focus to just the llvm/libcxx code for the time being, we should have a 
common idiom for resolving this problem.  It's likely to show up elsewhere too. 
 The fact that we were considering ifdefs here versus the pragmas in the 
earlier commit is concerning.


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316941 - Implement LWG 3013 - some filesystem members should not be noexcept.

2017-10-30 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Mon Oct 30 11:59:59 2017
New Revision: 316941

URL: http://llvm.org/viewvc/llvm-project?rev=316941&view=rev
Log:
Implement LWG 3013 - some filesystem members should not be noexcept.

LWG 3013 points out that the constructors and increment members
of the directory iterators need to allocate, and therefore cannot
be marked noexcept.

It also points out that `is_empty` and `copy` likely need to allocate
as well, and as such can also not be noexcept.

This patch speculatively implements the resolution removing noexcept,
because libc++ does indeed have the possibility of throwing on allocation
failure.

Modified:
libcxx/trunk/include/experimental/filesystem

libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/ctor.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/increment.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/ctor.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy/copy.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.is_empty/is_empty.pass.cpp

Modified: libcxx/trunk/include/experimental/filesystem
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=316941&r1=316940&r2=316941&view=diff
==
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Mon Oct 30 11:59:59 2017
@@ -81,10 +81,10 @@
 path canonical(const path& p, const path& base, error_code& ec);
 
 void copy(const path& from, const path& to);
-void copy(const path& from, const path& to, error_code& ec) _NOEXCEPT;
+void copy(const path& from, const path& to, error_code& ec);
 void copy(const path& from, const path& to, copy_options options);
 void copy(const path& from, const path& to, copy_options options,
-   error_code& ec) _NOEXCEPT;
+   error_code& ec);
 
 bool copy_file(const path& from, const path& to);
 bool copy_file(const path& from, const path& to, error_code& ec) _NOEXCEPT;
@@ -1351,7 +1351,7 @@ void copy(const path& __from, const path
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-void copy(const path& __from, const path& __to, error_code& __ec) _NOEXCEPT {
+void copy(const path& __from, const path& __to, error_code& __ec) {
 __copy(__from, __to, copy_options::none, &__ec);
 }
 
@@ -1362,7 +1362,7 @@ void copy(const path& __from, const path
 
 inline _LIBCPP_INLINE_VISIBILITY
 void copy(const path& __from, const path& __to,
-  copy_options __opt, error_code& __ec) _NOEXCEPT {
+  copy_options __opt, error_code& __ec) {
 __copy(__from, __to, __opt, &__ec);
 }
 
@@ -1561,7 +1561,7 @@ bool is_empty(const path& __p) {
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-bool is_empty(const path& __p, error_code& __ec) _NOEXCEPT {
+bool is_empty(const path& __p, error_code& __ec) {
 return __fs_is_empty(__p, &__ec);
 }
 
@@ -1903,12 +1903,12 @@ public:
 : directory_iterator(__p, nullptr, __opts)
 { }
 
-directory_iterator(const path& __p, error_code& __ec) _NOEXCEPT
+directory_iterator(const path& __p, error_code& __ec)
 : directory_iterator(__p, &__ec)
 { }
 
 directory_iterator(const path& __p, directory_options __opts,
-   error_code& __ec) _NOEXCEPT
+   error_code& __ec)
 : directory_iterator(__p, &__ec, __opts)
 { }
 
@@ -1943,7 +1943,7 @@ public:
 return __p;
 }
 
-directory_iterator& increment(error_code& __ec) _NOEXCEPT
+directory_iterator& increment(error_code& __ec)
 { return __increment(&__ec); }
 
 private:
@@ -2013,12 +2013,12 @@ public:
 
 _LIBCPP_INLINE_VISIBILITY
 recursive_directory_iterator(const path& __p,
-directory_options __xoptions, error_code& __ec) _NOEXCEPT
+directory_options __xoptions, error_code& __ec)
 : recursive_directory_iterator(__p, __xoptions, &__ec)
 { }
 
 _LIBCPP_INLINE_VISIBILITY
-recursive_directory_iterator(const path& __p, error_code& __ec) _NOEXCEPT
+recursive_directory_iterator(const path& __p, error_code& __ec)
 : recursive_directory_iterator(__p, directory_options::none,  &__ec)
 { }
 
@@ -2060,7 +2060,7 @@ public:
 }
 
 _LIBCPP_INLINE_VISIBILITY
-recursive_directory_iterator& increment(error_code& __ec) _NOEXCEPT
+recursive_directory_iterator& increment(error_code& __ec)
 { return __increment(&__ec); }
 
 _LIBCPP_FUNC_VIS directory_options options() const;

Modified: 
libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/ctor.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-pr

[libunwind] r316942 - Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread Martin Storsjo via cfe-commits

Author: mstorsjo
Date: Mon Oct 30 12:06:34 2017
New Revision: 316942

URL: http://llvm.org/viewvc/llvm-project?rev=316942&view=rev
Log:
Change unw_word_t to always have the same size as the pointer size

This matches the original libunwind API. This also unifies the
type between ARM EHABI and the other configurations, and allows
getting rid of a number of casts in log messages.

The cursor size updates for ppc and or1k are untested, but
unw_proc_info_t shrinks by 4 uint64_t units on i386 at least.

Differential Revision: https://reviews.llvm.org/D39365

Modified:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/include/libunwind.h
libunwind/trunk/src/Unwind-EHABI.cpp
libunwind/trunk/src/UnwindLevel1-gcc-ext.c
libunwind/trunk/src/UnwindLevel1.c
libunwind/trunk/src/libunwind.cpp

Modified: libunwind/trunk/include/__libunwind_config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Mon Oct 30 12:06:34 2017
@@ -26,7 +26,7 @@
 # if defined(__i386__)
 #  define _LIBUNWIND_TARGET_I386
 #  define _LIBUNWIND_CONTEXT_SIZE 8
-#  define _LIBUNWIND_CURSOR_SIZE 19
+#  define _LIBUNWIND_CURSOR_SIZE 15
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_X86
 # elif defined(__x86_64__)
 #  define _LIBUNWIND_TARGET_X86_64 1
@@ -41,7 +41,7 @@
 # elif defined(__ppc__)
 #  define _LIBUNWIND_TARGET_PPC 1
 #  define _LIBUNWIND_CONTEXT_SIZE 117
-#  define _LIBUNWIND_CURSOR_SIZE 128
+#  define _LIBUNWIND_CURSOR_SIZE 124
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
 # elif defined(__aarch64__)
 #  define _LIBUNWIND_TARGET_AARCH64 1
@@ -61,7 +61,7 @@
 # elif defined(__or1k__)
 #  define _LIBUNWIND_TARGET_OR1K 1
 #  define _LIBUNWIND_CONTEXT_SIZE 16
-#  define _LIBUNWIND_CURSOR_SIZE 28
+#  define _LIBUNWIND_CURSOR_SIZE 24
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K
 # else
 #  error "Unsupported architecture."

Modified: libunwind/trunk/include/libunwind.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/libunwind.h?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/include/libunwind.h (original)
+++ libunwind/trunk/include/libunwind.h Mon Oct 30 12:06:34 2017
@@ -72,11 +72,10 @@ typedef struct unw_cursor_t unw_cursor_t
 typedef struct unw_addr_space *unw_addr_space_t;
 
 typedef int unw_regnum_t;
+typedef uintptr_t unw_word_t;
 #if defined(_LIBUNWIND_ARM_EHABI)
-typedef uint32_t unw_word_t;
 typedef uint64_t unw_fpreg_t;
 #else
-typedef uint64_t unw_word_t;
 typedef double unw_fpreg_t;
 #endif
 

Modified: libunwind/trunk/src/Unwind-EHABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-EHABI.cpp?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/src/Unwind-EHABI.cpp (original)
+++ libunwind/trunk/src/Unwind-EHABI.cpp Mon Oct 30 12:06:34 2017
@@ -14,6 +14,7 @@
 
 #if defined(_LIBUNWIND_ARM_EHABI)
 
+#include 
 #include 
 #include 
 #include 
@@ -468,11 +469,11 @@ unwind_phase1(unw_context_t *uc, unw_cur
   unw_word_t pc;
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase1(ex_ojb=%p): pc=0x%llX, start_ip=0x%llX, func=%s, "
-  "lsda=0x%llX, personality=0x%llX",
-  static_cast(exception_object), (long long)pc,
-  (long long)frameInfo.start_ip, functionName,
-  (long long)frameInfo.lsda, (long long)frameInfo.handler);
+  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIxPTR ", start_ip=0x%" PRIxPTR 
", func=%s, "
+  "lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR,
+  static_cast(exception_object), pc,
+  frameInfo.start_ip, functionName,
+  frameInfo.lsda, frameInfo.handler);
 }
 
 // If there is a personality routine, ask it if it will want to stop at
@@ -584,11 +585,11 @@ static _Unwind_Reason_Code unwind_phase2
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase2(ex_ojb=%p): start_ip=0x%llX, func=%s, sp=0x%llX, "
-  "lsda=0x%llX, personality=0x%llX",
-  static_cast(exception_object), (long long)frameInfo.start_ip,
-  functionName, (long long)sp, (long long)frameInfo.lsda,
-  (long long)frameInfo.handler);
+  "unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIxPTR ", func=%s, sp=0x%" 
PRIxPTR ", "
+  "lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR "",
+  static_cast(exception_object), frameInfo.start_ip,
+  functionName, sp, frameI

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316942: Change unw_word_t to always have the same size as 
the pointer size (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D39365?vs=120562&id=120860#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39365

Files:
  libunwind/trunk/include/__libunwind_config.h
  libunwind/trunk/include/libunwind.h
  libunwind/trunk/src/Unwind-EHABI.cpp
  libunwind/trunk/src/UnwindLevel1-gcc-ext.c
  libunwind/trunk/src/UnwindLevel1.c
  libunwind/trunk/src/libunwind.cpp

Index: libunwind/trunk/src/UnwindLevel1.c
===
--- libunwind/trunk/src/UnwindLevel1.c
+++ libunwind/trunk/src/UnwindLevel1.c
@@ -76,8 +76,8 @@
   unw_word_t pc;
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIx64 ", start_ip=0x%" PRIx64
-  ", func=%s, lsda=0x%" PRIx64 ", personality=0x%" PRIx64 "",
+  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIxPTR ", start_ip=0x%" PRIxPTR
+  ", func=%s, lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR "",
   (void *)exception_object, pc, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
@@ -170,9 +170,9 @@
  &offset) != UNW_ESUCCESS) ||
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
-  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIx64
- ", func=%s, sp=0x%" PRIx64 ", lsda=0x%" PRIx64
- ", personality=0x%" PRIx64,
+  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIxPTR
+ ", func=%s, sp=0x%" PRIxPTR ", lsda=0x%" PRIxPTR
+ ", personality=0x%" PRIxPTR,
  (void *)exception_object, frameInfo.start_ip,
  functionName, sp, frameInfo.lsda,
  frameInfo.handler);
@@ -213,8 +213,8 @@
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   unw_get_reg(cursor, UNW_REG_SP, &sp);
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): re-entering "
- "user code with ip=0x%" PRIx64
- ", sp=0x%" PRIx64,
+ "user code with ip=0x%" PRIxPTR
+ ", sp=0x%" PRIxPTR,
  (void *)exception_object, pc, sp);
 }
 unw_resume(cursor);
@@ -262,8 +262,8 @@
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase2_forced(ex_ojb=%p): start_ip=0x%" PRIx64
-  ", func=%s, lsda=0x%" PRIx64 ", personality=0x%" PRIx64,
+  "unwind_phase2_forced(ex_ojb=%p): start_ip=0x%" PRIxPTR
+  ", func=%s, lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR,
   (void *)exception_object, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
@@ -467,17 +467,17 @@
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_word_t result;
   unw_get_reg(cursor, index, &result);
-  _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%" PRIx64,
-   (void *)context, index, (uint64_t)result);
+  _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%" PRIxPTR,
+   (void *)context, index, result);
   return (uintptr_t)result;
 }
 
 /// Called by personality handler during phase 2 to alter register values.
 _LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context *context, int index,
  uintptr_t value) {
-  _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0" PRIx64
+  _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0" PRIxPTR
")",
-   (void *)context, index, (uint64_t)value);
+   (void *)context, index, value);
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_set_reg(cursor, index, value);
 }
@@ -487,18 +487,18 @@
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_word_t result;
   unw_get_reg(cursor, UNW_REG_IP, &result);
-  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
-   (void *)context, (uint64_t)result);
+  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIxPTR,
+   (void *)context, result);
   return (uintptr_t)result;
 }
 
 /// Called by personality handler during phase 2 to alter instruction pointer,
 /// such as setting where the landing pad is, so _Unwind_Resume() will
 /// start executing in the landing pad.
 _LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Cont

[PATCH] D39280: [libunwind] Use uint64_t for unw_word_t in ARM EHABI

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo abandoned this revision.
mstorsjo added a comment.

This was made obsolete by https://reviews.llvm.org/D39365.


https://reviews.llvm.org/D39280



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

2017-10-30 Thread George Karpenkov via Phabricator via cfe-commits

george.karpenkov updated this revision to Diff 120861.

https://reviews.llvm.org/D39428

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/BodyFarm.h
  lib/Analysis/AnalysisDeclContext.cpp


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(MD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(MD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -304,10 +305,8 @@
   return AC.get();
 }
 
-BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
-  if (!FunctionBodyFarm)
-FunctionBodyFarm = llvm::make_unique(ASTCtx, Injector.get());
-  return FunctionBodyFarm.get();
+BodyFarm& AnalysisDeclContextManager::getBodyFarm() {
+  return FunctionBodyFarm;
 }
 
 const StackFrameContext *
Index: include/clang/Analysis/BodyFarm.h
===
--- include/clang/Analysis/BodyFarm.h
+++ include/clang/Analysis/BodyFarm.h
@@ -39,6 +39,9 @@
   /// Factory method for creating bodies for Objective-C properties.
   Stmt *getBody(const ObjCMethodDecl *D);
 
+  /// Remove copy constructor to avoid accidental copying.
+  BodyFarm(const BodyFarm& other) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -419,9 +419,9 @@
   /// declarations from external source.
   std::unique_ptr Injector;
 
-  /// Pointer to a factory for creating and caching implementations for common
+  /// A factory for creating and caching implementations for common
   /// methods during the analysis.
-  std::unique_ptr FunctionBodyFarm;
+  BodyFarm FunctionBodyFarm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -475,8 +475,8 @@
 return LocContexts.getStackFrame(getContext(D), Parent, S, Blk, Idx);
   }
 
-  /// Get and lazily create a {@code BodyFarm} instance.
-  BodyFarm *getBodyFarm();
+  /// Get a reference to {@code BodyFarm} instance.
+  BodyFarm& getBodyFarm();
 
   /// Discard all previously created AnalysisDeclContexts.
   void clear();


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm(

[PATCH] D39251: [libunwind] Fix building for ARM with dwarf exception handling

2017-10-30 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo updated this revision to Diff 120863.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Avoiding duplicating the highest register number, avoiding any further casts in 
log lines.


https://reviews.llvm.org/D39251

Files:
  include/__libunwind_config.h
  include/libunwind.h
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -55,7 +55,7 @@
 # define REGISTER_KIND Registers_ppc
 #elif defined(__aarch64__)
 # define REGISTER_KIND Registers_arm64
-#elif defined(_LIBUNWIND_ARM_EHABI)
+#elif defined(__arm__)
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -583,6 +583,12 @@
   }
 #endif
 
+#if defined(_LIBUNWIND_TARGET_ARM)
+  compact_unwind_encoding_t dwarfEncoding(Registers_arm &) const {
+return 0;
+  }
+#endif
+
 #if defined (_LIBUNWIND_TARGET_OR1K)
   compact_unwind_encoding_t dwarfEncoding(Registers_or1k &) const {
 return 0;
Index: src/Registers.hpp
===
--- src/Registers.hpp
+++ src/Registers.hpp
@@ -1386,7 +1386,7 @@
   Registers_arm(const void *registers);
 
   boolvalidRegister(int num) const;
-  uint32_tgetRegister(int num);
+  uint32_tgetRegister(int num) const;
   voidsetRegister(int num, uint32_t value);
   boolvalidFloatRegister(int num) const;
   unw_fpreg_t getFloatRegister(int num);
@@ -1399,6 +1399,7 @@
 restoreSavedFloatRegisters();
 restoreCoreAndJumpTo();
   }
+  static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM; }
 
   uint32_t  getSP() const { return _registers.__sp; }
   void  setSP(uint32_t value) { _registers.__sp = value; }
@@ -1472,11 +1473,11 @@
   // Whether iWMMX data registers are saved.
   bool _saved_iwmmx;
   // Whether iWMMX control registers are saved.
-  bool _saved_iwmmx_control;
+  mutable bool _saved_iwmmx_control;
   // iWMMX registers
   unw_fpreg_t _iwmmx[16];
   // iWMMX control registers
-  uint32_t _iwmmx_control[4];
+  mutable uint32_t _iwmmx_control[4];
 #endif
 };
 
@@ -1533,7 +1534,7 @@
   return false;
 }
 
-inline uint32_t Registers_arm::getRegister(int regNum) {
+inline uint32_t Registers_arm::getRegister(int regNum) const {
   if (regNum == UNW_REG_SP || regNum == UNW_ARM_SP)
 return _registers.__sp;
 
Index: include/libunwind.h
===
--- include/libunwind.h
+++ include/libunwind.h
@@ -73,7 +73,7 @@
 
 typedef int unw_regnum_t;
 typedef uintptr_t unw_word_t;
-#if defined(_LIBUNWIND_ARM_EHABI)
+#if defined(__arm__)
 typedef uint64_t unw_fpreg_t;
 #else
 typedef double unw_fpreg_t;
Index: include/__libunwind_config.h
===
--- include/__libunwind_config.h
+++ include/__libunwind_config.h
@@ -19,7 +19,7 @@
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86_6432
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC   112
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64 95
-#define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM   95
+#define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM   287
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K  31
 
 #if defined(_LIBUNWIND_IS_NATIVE_ONLY)
@@ -75,7 +75,7 @@
 # define _LIBUNWIND_TARGET_OR1K 1
 # define _LIBUNWIND_CONTEXT_SIZE 128
 # define _LIBUNWIND_CURSOR_SIZE 140
-# define _LIBUNWIND_HIGHEST_DWARF_REGISTER 119
+# define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287
 #endif // _LIBUNWIND_IS_NATIVE_ONLY
 
 #endif // LIBUNWIND_CONFIG_H__
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316948 - [analyzer] [tests] Remove empty folders in reference results, do not store diffs.txt

2017-10-30 Thread George Karpenkov via cfe-commits

Author: george.karpenkov
Date: Mon Oct 30 12:40:33 2017
New Revision: 316948

URL: http://llvm.org/viewvc/llvm-project?rev=316948&view=rev
Log:
[analyzer] [tests] Remove empty folders in reference results, do not store 
diffs.txt

Storing diffs.txt is now redundant, as we simply dump the CmpRuns output
to stdout (it is saved in CI and tends to be small).
Not generating those files enables us to remove empty folders, which
confuse git, as it would not add them with reference results.

Modified:
cfe/trunk/utils/analyzer/SATestBuild.py
cfe/trunk/utils/analyzer/SATestUpdateDiffs.py

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=316948&r1=316947&r2=316948&view=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Mon Oct 30 12:40:33 2017
@@ -120,8 +120,6 @@ BuildLogName = "run_static_analyzer.log"
 # displayed when buildbot detects a build failure.
 NumOfFailuresInSummary = 10
 FailuresSummaryFileName = "failures.txt"
-# Summary of the result diffs.
-DiffsSummaryFileName = "diffs.txt"
 
 # The scan-build result directory.
 SBOutputDirName = "ScanBuildResults"
@@ -434,6 +432,16 @@ def CleanUpEmptyPlists(SBOutputDir):
 continue
 
 
+def CleanUpEmptyFolders(SBOutputDir):
+"""
+Remove empty folders from results, as git would not store them.
+"""
+Subfolders = glob.glob(SBOutputDir + "/*")
+for Folder in Subfolders:
+if not os.listdir(Folder):
+os.removedirs(Folder)
+
+
 def checkBuild(SBOutputDir):
 """
 Given the scan-build output directory, checks if the build failed
@@ -446,6 +454,7 @@ def checkBuild(SBOutputDir):
 TotalFailed = len(Failures)
 if TotalFailed == 0:
 CleanUpEmptyPlists(SBOutputDir)
+CleanUpEmptyFolders(SBOutputDir)
 Plists = glob.glob(SBOutputDir + "/*/*.plist")
 print "Number of bug reports (non-empty plist files) produced: %d" %\
 len(Plists)
@@ -519,9 +528,8 @@ def runCmpResults(Dir, Strictness=0):
 if Verbose == 1:
 print "  Comparing Results: %s %s" % (RefDir, NewDir)
 
-DiffsPath = os.path.join(NewDir, DiffsSummaryFileName)
 PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName)
-Opts = CmpRuns.CmpOptions(DiffsPath, "", PatchedSourceDirPath)
+Opts = CmpRuns.CmpOptions(rootA="", rootB=PatchedSourceDirPath)
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
 CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)

Modified: cfe/trunk/utils/analyzer/SATestUpdateDiffs.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestUpdateDiffs.py?rev=316948&r1=316947&r2=316948&view=diff
==
--- cfe/trunk/utils/analyzer/SATestUpdateDiffs.py (original)
+++ cfe/trunk/utils/analyzer/SATestUpdateDiffs.py Mon Oct 30 12:40:33 2017
@@ -54,22 +54,9 @@ def updateReferenceResults(ProjName, Pro
 # Clean up the generated difference results.
 SATestBuild.cleanupReferenceResults(RefResultsPath)
 
-# Remove the created .diffs file before adding.
-removeDiffsSummaryFiles(RefResultsPath)
-
 runCmd('git add "%s"' % (RefResultsPath,))
 
 
-def removeDiffsSummaryFiles(RefResultsPath):
-"""
-Remove all auto-generated .diffs files in reference data.
-"""
-for (Dirpath, Dirnames, Filenames) in os.walk(RefResultsPath):
-if SATestBuild.DiffsSummaryFileName in Filenames:
-runCmd("rm '%s'" % os.path.join(
-Dirpath, SATestBuild.DiffsSummaryFileName))
-
-
 def main(argv):
 if len(argv) == 2 and argv[1] in ('-h', '--help'):
 print >> sys.stderr, "Update static analyzer reference results based "\


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316951 - Add more fuzzing bits: partial_sort_copy, partition_copy, unique, unique_copy. No functional change to libc++; this is all test infastructure

2017-10-30 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Mon Oct 30 12:51:58 2017
New Revision: 316951

URL: http://llvm.org/viewvc/llvm-project?rev=316951&view=rev
Log:
Add more fuzzing bits: partial_sort_copy, partition_copy, unique, unique_copy. 
No functional change to libc++; this is all test infastructure

Added:
libcxx/trunk/test/libcxx/fuzzing/partial_sort_copy.cpp
libcxx/trunk/test/libcxx/fuzzing/partition_copy.cpp
libcxx/trunk/test/libcxx/fuzzing/unique.cpp
libcxx/trunk/test/libcxx/fuzzing/unique_copy.cpp
Modified:
libcxx/trunk/fuzzing/RoutineNames.txt
libcxx/trunk/fuzzing/fuzzing.cpp
libcxx/trunk/fuzzing/fuzzing.h

Modified: libcxx/trunk/fuzzing/RoutineNames.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/fuzzing/RoutineNames.txt?rev=316951&r1=316950&r2=316951&view=diff
==
--- libcxx/trunk/fuzzing/RoutineNames.txt (original)
+++ libcxx/trunk/fuzzing/RoutineNames.txt Mon Oct 30 12:51:58 2017
@@ -1,9 +1,13 @@
 sort
 stable_sort
 partition
+partition_copy
 stable_partition
+unique
+unique_copy
 nth_element
 partial_sort
+partial_sort_copy
 make_heap
 push_heap
 pop_heap

Modified: libcxx/trunk/fuzzing/fuzzing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/fuzzing/fuzzing.cpp?rev=316951&r1=316950&r2=316951&view=diff
==
--- libcxx/trunk/fuzzing/fuzzing.cpp (original)
+++ libcxx/trunk/fuzzing/fuzzing.cpp Mon Oct 30 12:51:58 2017
@@ -102,11 +102,13 @@ struct is_even
}
 };
 
-// == sort ==
+typedef std::vector Vec;
+typedef std::vector StableVec;
 
+// == sort ==
 int sort(const uint8_t *data, size_t size)
 {
-   std::vector working(data, data + size);
+   Vec working(data, data + size);
std::sort(working.begin(), working.end());
 
if (!std::is_sorted(working.begin(), working.end())) return 1;
@@ -116,13 +118,12 @@ int sort(const uint8_t *data, size_t siz
 
 
 // == stable_sort ==
-
 int stable_sort(const uint8_t *data, size_t size)
 {
-   std::vector input;
+   StableVec input;
for (size_t i = 0; i < size; ++i)
input.push_back(stable_test(data[i], i));
-   std::vector working = input;
+   StableVec working = input;
std::stable_sort(working.begin(), working.end(), key_less());
 
if (!std::is_sorted(working.begin(), working.end(), key_less()))   
return 1;
@@ -138,10 +139,9 @@ int stable_sort(const uint8_t *data, siz
 }
 
 // == partition ==
-
 int partition(const uint8_t *data, size_t size)
 {
-   std::vector working(data, data + size);
+   Vec working(data, data + size);
auto iter = std::partition(working.begin(), working.end(), 
is_even());
 
if (!std::all_of (working.begin(), iter, is_even())) return 1;
@@ -151,14 +151,38 @@ int partition(const uint8_t *data, size_
 }
 
 
-// == stable_partition ==
+// == partition_copy ==
+int partition_copy(const uint8_t *data, size_t size)
+{
+   Vec v1, v2;
+   auto iter = std::partition_copy(data, data + size,
+   std::back_inserter(v1), std::back_inserter(v2),
+   is_even());
+
+// The two vectors should add up to the original size
+   if (v1.size() + v2.size() != size) return 1;
+
+// All of the even values should be in the first vector, and none in the 
second
+   if (!std::all_of (v1.begin(), v1.end(), is_even())) return 2;
+   if (!std::none_of(v2.begin(), v2.end(), is_even())) return 3;
+
+// Every value in both vectors has to be in the original
+   for (auto v: v1)
+   if (std::find(data, data + size, v) == data + size) return 4;
+   
+   for (auto v: v2)
+   if (std::find(data, data + size, v) == data + size) return 5;
+   
+   return 0;
+}
 
+// == stable_partition ==
 int stable_partition (const uint8_t *data, size_t size)
 {
-   std::vector input;
+   StableVec input;
for (size_t i = 0; i < size; ++i)
input.push_back(stable_test(data[i], i));
-   std::vector working = input;
+   StableVec working = input;
auto iter = std::stable_partition(working.begin(), working.end(), 
is_even());
 
if (!std::all_of (working.begin(), iter, is_even())) 
return 1;
@@ -175,7 +199,7 @@ int nth_element (const uint8_t *data, si
 {
if (size <= 1) return 0;
const size_t partition_point = data[0] % size;  
-   std::vector working(data + 1, data + size);
+   Vec working(data + 1, data + size);
const auto partition_iter = working.begin() + partition_point;
std::nth_element(working.begin(), partition_iter, working.end());
 
@@ -203,7 +227,7 @@ int partial_sort (const uint8_t *data, s
 {
if (size <= 1) return 0;
const size_t sort_point = data[0] % size;
-   std::vector working(data + 1, data + size);
+   Vec working(data + 1, da

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

In https://reviews.llvm.org/D39149#910936, @smeenai wrote:

> I'm thinking you could account for all possible type sizes in the existing 
> (enabled by default) warning, and have a different warning for possibly 
> tautological comparisons. E.g. if a `long` is being compared against 
> `INT_MAX`, you know that's only tautological on some platforms, so it should 
> go under `-Wpossible-tautological-constant-compare` (which would only be 
> enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> `-Wtautological-constant-compare` should be reserved for definitely 
> tautological cases.


This sounds like a very promising direction. (That is: if the types of the 
values being compared are different, but are of the same size, then suppress 
the warning or move it to a different warning group that's not part of 
`-Wtautological-compare`.) That would also suppress warnings for cases like 'ch 
> CHAR_MAX', when `ch` is a `char`, but we could detect and re-enable the 
warning for such cases by, say, always warning if the constant side is an 
integer literal.


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/D39149#911024, @rsmith wrote:

> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing 
> > (enabled by default) warning, and have a different warning for possibly 
> > tautological comparisons. E.g. if a `long` is being compared against 
> > `INT_MAX`, you know that's only tautological on some platforms, so it 
> > should go under `-Wpossible-tautological-constant-compare` (which would 
> > only be enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> > `-Wtautological-constant-compare` should be reserved for definitely 
> > tautological cases.
>
>
> This sounds like a very promising direction. (




> That is: if the types of the values being compared are different, but are of 
> the same size, then suppress the warning or move it to a different warning 
> group that's not part of `-Wtautological-compare`.

Ok, now **that** makes at least //some// sense :)

> ) That would also suppress warnings for cases like 'ch > CHAR_MAX', when `ch` 
> is a `char`, but we could detect and re-enable the warning for such cases by, 
> say, always warning if the constant side is an integer literal.




https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39149: [libc++] Prevent tautological comparisons

2017-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D39149#911024, @rsmith wrote:

> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing 
> > (enabled by default) warning, and have a different warning for possibly 
> > tautological comparisons. E.g. if a `long` is being compared against 
> > `INT_MAX`, you know that's only tautological on some platforms, so it 
> > should go under `-Wpossible-tautological-constant-compare` (which would 
> > only be enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> > `-Wtautological-constant-compare` should be reserved for definitely 
> > tautological cases.
>
>
> This sounds like a very promising direction. (That is: if the types of the 
> values being compared are different, but are of the same size, then suppress 
> the warning or move it to a different warning group that's not part of 
> `-Wtautological-compare`.) That would also suppress warnings for cases like 
> 'ch > CHAR_MAX', when `ch` is a `char`, but we could detect and re-enable the 
> warning for such cases by, say, always warning if the constant side is an 
> integer literal.


Yeah, the type comparison implementation was what I was thinking of originally, 
though I wasn't sure about the edge cases.

When you say "always warning if the constant side is an integer literal", do 
you mean if it's a straight-up integer literal, or if it's an expression which 
evaluates to an integer literal at compile time? For example, would there be a 
difference if you were comparing to `numeric_limits::max()` vs. `INT_MAX`?


https://reviews.llvm.org/D39149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36101: Fix usage of right shift operator in fold expressions

2017-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: lib/Parse/ParseExpr.cpp:273
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }

Do we ever need to pass in the `Parser`'s `GreaterThanIsOperator` value here? 
(From a quick check, it looks like we never call `isFoldOperator` in any case 
where `>` might end a template argument list instead, but it's not completely 
obvious, because we do support parsing unparenthesized fold operators for 
better error handling in some cases.)

Seems like it would be safer to pass the correct value in here, even if it 
happens to not matter right now.


https://reviews.llvm.org/D36101



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39430: [clangd] formatting: don't ignore style

2017-10-30 Thread Raoul Wols via Phabricator via cfe-commits

rwols created this revision.

Look up in parent directories for a .clang-format file. Use "LLVM" as fallback 
style.


https://reviews.llvm.org/D39430

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h

Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -285,11 +285,15 @@
   llvm::Optional switchSourceHeader(PathRef Path);
 
   /// Run formatting for \p Rng inside \p File.
-  std::vector formatRange(PathRef File, Range Rng);
+  llvm::Expected> formatRange(PathRef File,
+Range Rng);
+
   /// Run formatting for the whole \p File.
-  std::vector formatFile(PathRef File);
+  llvm::Expected> formatFile(PathRef File);
+
   /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector formatOnType(PathRef File, Position Pos);
+  llvm::Expected> formatOnType(PathRef File,
+ Position Pos);
 
   /// Gets current document contents for \p File. \p File must point to a
   /// currently tracked file.
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -34,13 +34,14 @@
   std::promise &Promise;
 };
 
-std::vector formatCode(StringRef Code, StringRef Filename,
- ArrayRef Ranges) {
+llvm::Expected>
+formatCode(StringRef Code, StringRef Filename,
+   ArrayRef Ranges) {
   // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
+  auto StyleOrError = format::getStyle("file", Filename, "LLVM");
+  if (!StyleOrError)
+return StyleOrError.takeError();
+  auto Result = format::reformat(StyleOrError.get(), Code, Ranges, Filename);
   return std::vector(Result.begin(), Result.end());
 }
 
@@ -301,23 +302,24 @@
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector ClangdServer::formatRange(PathRef File,
-Range Rng) {
+llvm::Expected>
+ClangdServer::formatRange(PathRef File, Range Rng) {
   std::string Code = getDocument(File);
 
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector ClangdServer::formatFile(PathRef File) {
+llvm::Expected>
+ClangdServer::formatFile(PathRef File) {
   // Format everything.
   std::string Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector ClangdServer::formatOnType(PathRef File,
- Position Pos) {
+llvm::Expected>
+ClangdServer::formatOnType(PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
   std::string Code = getDocument(File);
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -73,6 +73,12 @@
   std::vector
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
+  void replyWithTextEditsOrError(
+  Ctx C, std::string Code,
+  llvm::Expected>
+  ReplacementsOrError,
+  llvm::StringRef MessageInCaseError) const;
+
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
+#include 
 
 using namespace clang::clangd;
 using namespace clang;
@@ -89,30 +90,40 @@
   Server.removeDocument(Params.textDocument.uri.file);
 }
 
+void ClangdLSPServer::replyWithTextEditsOrError(
+Ctx C, std::string Code,
+llvm::Expected>
+ReplacementsOrError,
+llvm::StringRef MessageInCaseError) const {
+  if (!ReplacementsOrError) {
+C.replyError(/*UnknownErrorCode*/ -32001, MessageInCaseError);
+  } else {
+C.reply("[" + replacementsToEdits(Code, ReplacementsOrError.get()) + "]");
+  }
+}
+
 void ClangdLSPServer::onDocumentOnTypeFormatting(
 Ctx C, DocumentOnTypeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  std::string Edits =
-  replacementsToEdits(Code, Server.formatOnType(File, Params.position));
-  C.reply("[" + Edits + "]");
+  const auto File = Params.textDocument.uri.file;
+  replyWithTextEditsOrError(std::move(C), Server.getDocument(File),
+

[PATCH] D39430: [clangd] formatting: don't ignore style

2017-10-30 Thread Raoul Wols via Phabricator via cfe-commits

rwols added a comment.

Does anyone have an idea how I would write a test for this?

Also, I should have commit access after approval now. So that's less work for 
you guys ;)


https://reviews.llvm.org/D39430



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

1 2 >

1 - 100 of 133 matches

Mail list logo