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

[Balogh , Ádám via Phabricator via cfe-commits]

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

Updated according to the comments.


https://reviews.llvm.org/D39121

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  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
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
  test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp

Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+
+namespace std {
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+size_t strlen(const char *);
+} // namespace std
+
+namespace non_std {
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+size_t strlen(const char *);
+} // namespace non_std
+
+void bad_std_malloc_std_strlen(char *name) {
+  char *new_name = (char *)std::malloc(std::strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char \*\)std::malloc\(}}std::strlen(name) + 1{{\);$}}
+}
+
+void ignore_non_std_malloc_std_strlen(char *name) {
+  char *new_name = (char *)non_std::malloc(std::strlen(name + 1));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // Ignore functions of the malloc family in custom namespaces
+}
+
+void ignore_std_malloc_non_std_strlen(char *name) {
+  char *new_name = (char *)std::malloc(non_std::strlen(name + 1));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // Ignore functions of the strlen family in custom namespaces
+}
Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+
+size_t strlen(const char *);
+size_t strnlen(const char *, size_t);
+size_t strnlen_s(const char *, size_t);
+
+typedef unsigned wchar_t;
+
+size_t wcslen(const wchar_t *);
+size_t wcsnlen(const wchar_t *, size_t);
+size_t wcsnlen_s(const wchar_t *, size_t);
+
+void bad_malloc(char *name) {
+  char *new_name = (char *)malloc(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char \*\)malloc\(}}strlen(name) + 1{{\);$}}
+  new_name = (char *)malloc(strnlen(name + 1, 10));
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: addition operator is applied to the argument of strnlen
+  // CHECK-FIXES: {{^  new_name = \(char \*\)malloc\(}}strnlen(name, 10) + 1{{\);$}}
+  new_name = (char *)malloc(strnlen_s(name + 1, 10));
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: addition operator is applied to the argument of strnlen_s
+  // CHECK-FIXES: {{^  new_name = \(char \*\)malloc\(}}strnlen_s(name, 10) + 1{{\);$}}
+}
+
+void bad_malloc_wide(wchar_t *name) {
+  wchar_t *new_name = (wchar_t *)malloc(wcslen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: addition operator is applied to the argument of wcslen
+  // CHECK-FIXES: {{^  wchar_t \*new_name = \(wchar_t \*\)malloc\(}}wcslen(name) + 1{{\);$}}
+  new_name = (wchar_t *)malloc(wcsnlen(name + 1, 10));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: addition operator is applied to the argument of wcsnlen
+  // CHECK-FIXES: {{^  new_name = \(wchar_t \*\)malloc\(}}wcsnlen(name, 10) + 1{{\);$}}
+  new_name = (wchar_t *)malloc(wcsnlen_s(name + 1, 10));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: addition operator is applied to the argument of wcsnlen_s
+  // CHECK-FIXES: {{^  new_name = \(wchar_t \*\)malloc\(}}wcsnlen_s(name, 10) + 1{{\);$}}
+}
+
+void bad_alloca(char *name) {
+  char *new_name = (char *)alloca(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char \*\)alloca\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_calloc(char *name) {
+  char *new_names = (char *)calloc(2, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_names = \(char \*\)

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

[Balogh , Ádám via Phabricator via cfe-commits]

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

Updated to match https://reviews.llvm.org/D39121.


https://reviews.llvm.org/D39367

Files:
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
  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
@@ -31,3 +31,29 @@
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
   // 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
@@ -6,12 +6,12 @@
 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.
+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,6 +28,24 @@
   char *c = (char*) malloc(strlen(str) + 1);
 
 
+`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 diagnostic:
 
 .. code-block:: c
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,7 +64,7 @@
   ``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()``).
+  ``alloca()``) or the ``new[]`` operator in `C++`.
 
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
===
--- clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -48,26 +48,32 @@
   const auto Alloc1Func =
   functionDecl(anyOf(hasName("::calloc"), hasName("std::calloc"),
  hasName("::realloc"), hasName("std::realloc")));
-
   Finder->addMatcher(
   callExpr(callee(Alloc0Func), hasArgument(0, BadArg)).bind("Alloc"), this);
   Finder->addMatcher(
   callExpr(callee(Alloc1Func), hasArgument(1, BadArg)).bind("Alloc"), this);
+  Finder->addMatcher(
+  cxxNewExpr(isArray(), hasArraySize(BadArg)).bind("Alloc"), this);
 }
 
 void MisplacedOperatorInStrlenInAllocCheck::check(
 const MatchFinder::MatchResult &Result) {
-  con

[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

[Balogh , Ádám via Phabricator via cfe-commits]

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

Updated to match https://reviews.llvm.org/D39121.


https://reviews.llvm.org/D39370

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


Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
===
--- test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
@@ -75,3 +75,11 @@
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is 
applied to the argument of strlen
   // If expression is in extra parentheses, consider it as intentional
 }
+
+void (*(*const alloc_ptr)(size_t)) = malloc;
+
+void bad_indirect_alloc(char *name) {
+  char *new_name = (char *)alloc_ptr(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: addition operator is applied to 
the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char \*\)alloc_ptr\(}}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
@@ -7,9 +7,10 @@
 ``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.
+The check detects error cases even if one of these functions is called by a
+constant function pointer.  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:
 
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
===
--- clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -49,10 +49,23 @@
   functionDecl(anyOf(hasName("::calloc"), hasName("std::calloc"),
  hasName("::realloc"), hasName("std::realloc")));
 
-  Finder->addMatcher(
-  callExpr(callee(Alloc0Func), hasArgument(0, BadArg)).bind("Alloc"), 
this);
-  Finder->addMatcher(
-  callExpr(callee(Alloc1Func), hasArgument(1, BadArg)).bind("Alloc"), 
this);
+  const auto Alloc0FuncPtr =
+  varDecl(hasType(isConstQualified()),
+  hasInitializer(ignoringParenImpCasts(
+  declRefExpr(hasDeclaration(Alloc0Func);
+  const auto Alloc1FuncPtr =
+  varDecl(hasType(isConstQualified()),
+  hasInitializer(ignoringParenImpCasts(
+  declRefExpr(hasDeclaration(Alloc1Func);
+
+  Finder->addMatcher(callExpr(callee(decl(anyOf(Alloc0Func, Alloc0FuncPtr))),
+  hasArgument(0, BadArg))
+ .bind("Alloc"),
+ this);
+  Finder->addMatcher(callExpr(callee(decl(anyOf(Alloc1Func, Alloc1FuncPtr))),
+  hasArgument(1, BadArg))
+ .bind("Alloc"),
+ this);
 }
 
 void MisplacedOperatorInStrlenInAllocCheck::check(


Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
===
--- test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c
@@ -75,3 +75,11 @@
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
   // If expression is in extra parentheses, consider it as intentional
 }
+
+void (*(*const alloc_ptr)(size_t)) = malloc;
+
+void bad_indirect_alloc(char *name) {
+  char *new_name = (char *)alloc_ptr(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char \*\)alloc_ptr\(}}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
@@ -7,9 +7,10 @@
 ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()``, and ``wcsnlen_

[PATCH] D39603: [clang-tidy] Support relative paths in run-clang-tidy.py

[Haojian Wu via Phabricator via cfe-commits]

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

The code looks good to me.

> I did not find any existing tests.

Yeah, we don't have good test strategy for these py files. We normally verify 
them by running them locally. I think we can land it as long as you test it 
locally (would be better to describe how you test it in the commit message).




Comment at: clang-tidy/tool/run-clang-tidy.py:232
+  files = [make_absolute(entry['file'], entry['directory'])
+  for entry in database]
 

nit: the code indention seems wrong here.


https://reviews.llvm.org/D39603



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

[PATCH] D39675: [clang-refactor] Use ClangTool more explicitly by making refaroing actions AST frontend actions.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.

This is a refactoring change. NFC


https://reviews.llvm.org/D39675

Files:
  tools/clang-refactor/ClangRefactor.cpp

Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -25,6 +25,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -256,9 +257,9 @@
   RefactoringActionRules ActionRules,
   cl::OptionCategory &Category)
   : SubCommand(Action->getCommand(), Action->getDescription()),
-Action(std::move(Action)), ActionRules(std::move(ActionRules)) {
+Action(std::move(Action)), ActionRules(std::move(ActionRules)),
+HasSelection(false) {
 // Check if the selection option is supported.
-bool HasSelection = false;
 for (const auto &Rule : this->ActionRules) {
   if ((HasSelection = Rule->hasSelectionRequirement()))
 break;
@@ -295,6 +296,9 @@
 return false;
   }
 
+  // Whether the selection is suppoted by any rule in the subcommand.
+  bool hasSelection() const { return HasSelection; }
+
   SourceSelectionArgument *getSelection() const {
 assert(Selection && "selection not supported!");
 return ParsedSelection.get();
@@ -310,11 +314,13 @@
   std::unique_ptr> Selection;
   std::unique_ptr ParsedSelection;
   RefactoringActionCommandLineOptions Options;
+  // Whether the selection is suppoted by any rule in the subcommand.
+  bool HasSelection;
 };
 
 class ClangRefactorConsumer final : public ClangRefactorToolConsumerInterface {
 public:
-  ClangRefactorConsumer() {}
+  ClangRefactorConsumer(AtomicChanges &Changes) : SourceChanges(&Changes) {}
 
   void handleError(llvm::Error Err) override {
 Optional Diag = DiagnosticError::take(Err);
@@ -329,24 +335,23 @@
   }
 
   void handle(AtomicChanges Changes) override {
-SourceChanges.insert(SourceChanges.begin(), Changes.begin(), Changes.end());
+SourceChanges->insert(SourceChanges->begin(), Changes.begin(),
+  Changes.end());
   }
 
   void handle(SymbolOccurrences Occurrences) override {
 llvm_unreachable("symbol occurrence results are not handled yet");
   }
 
-  const AtomicChanges &getSourceChanges() const { return SourceChanges; }
-
 private:
-  AtomicChanges SourceChanges;
+  AtomicChanges *SourceChanges;
 };
 
 class ClangRefactorTool {
 public:
-  std::vector> SubCommands;
-
-  ClangRefactorTool() {
+  ClangRefactorTool()
+  : SelectedSubcommand(nullptr), MatchingRule(nullptr),
+Consumer(new ClangRefactorConsumer(Changes)), HasFailed(false) {
 std::vector> Actions =
 createRefactoringActions();
 
@@ -369,59 +374,110 @@
 }
   }
 
-  using TUCallbackType = llvm::function_ref;
+  // Initializes the selected subcommand and refactoring rule based on the
+  // command line options.
+  llvm::Error Init() {
+auto Subcommand = getSelectedSubcommand();
+if (!Subcommand)
+  return Subcommand.takeError();
+auto Rule = getMatchingRule(**Subcommand);
+if (!Rule)
+  return Rule.takeError();
+
+SelectedSubcommand = *Subcommand;
+MatchingRule = *Rule;
+
+return llvm::Error::success();
+  }
+
+  bool hasFailed() const { return HasFailed; }
+
+  using TUCallbackType = std::function;
+
+  // Callback of an AST action. This invokes the matching rule on the given AST.
+  void callback(ASTContext &AST) {
+assert(SelectedSubcommand && MatchingRule && Consumer);
+RefactoringRuleContext Context(AST.getSourceManager());
+Context.setASTContext(AST);
+
+// If the selection option is test specific, we use a test-specific
+// consumer.
+std::unique_ptr TestConsumer;
+if (SelectedSubcommand->hasSelection())
+  TestConsumer = SelectedSubcommand->getSelection()->createCustomConsumer();
+ClangRefactorToolConsumerInterface *ActiveConsumer =
+TestConsumer ? TestConsumer.get() : Consumer.get();
+ActiveConsumer->beginTU(AST);
+// FIXME (Alex L): Implement non-selection based invocation path.
+if (SelectedSubcommand->hasSelection()) {
+  assert(SelectedSubcommand->getSelection() &&
+ "Missing selection argument?");
+  if (opts::Verbose)
+SelectedSubcommand->getSelection()->print(llvm::outs());
+  if (SelectedSubcommand->getSelection()->forAllRanges(
+  Context.getSources(), [&](SourceRange R) {
+Context.setSelectionRange(R);
+if (opts::Verbose)
+  logInvocation(*SelectedSubcommand, Context);
+MatchingRule->invoke(*ActiveConsumer, Context);
+  }))
+HasFailed = true;
+  ActiveConsumer->endTU();
+  return;
+}
+ActiveConsumer->endTU();
+  }
 
-  /// Parse

r317466 - [Tooling] Test internal::createExecutorFromCommandLineArgsImpl instead of the wrapper.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Nov  6 01:29:09 2017
New Revision: 317466

URL: http://llvm.org/viewvc/llvm-project?rev=317466&view=rev
Log:
[Tooling] Test internal::createExecutorFromCommandLineArgsImpl instead of the 
wrapper.

Modified:
cfe/trunk/unittests/Tooling/ExecutionTest.cpp

Modified: cfe/trunk/unittests/Tooling/ExecutionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ExecutionTest.cpp?rev=317466&r1=317465&r2=317466&view=diff
==
--- cfe/trunk/unittests/Tooling/ExecutionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ExecutionTest.cpp Mon Nov  6 01:29:09 2017
@@ -133,8 +133,8 @@ llvm::cl::OptionCategory TestCategory("e
 TEST(CreateToolExecutorTest, FailedCreateExecutorUndefinedFlag) {
   std::vector argv = {"prog", "--fake_flag_no_no_no", "f"};
   int argc = argv.size();
-  auto Executor =
-  createExecutorFromCommandLineArgs(argc, &argv[0], TestCategory);
+  auto Executor = internal::createExecutorFromCommandLineArgsImpl(
+  argc, &argv[0], TestCategory);
   ASSERT_FALSE((bool)Executor);
   llvm::consumeError(Executor.takeError());
 }
@@ -148,8 +148,8 @@ TEST(CreateToolExecutorTest, RegisterFla
 
   std::vector argv = {"prog", "--before_reset=set", "f"};
   int argc = argv.size();
-  auto Executor =
-  createExecutorFromCommandLineArgs(argc, &argv[0], TestCategory);
+  auto Executor = internal::createExecutorFromCommandLineArgsImpl(
+  argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
   EXPECT_EQ(BeforeReset, "set");
   BeforeReset.removeArgument();
@@ -158,8 +158,8 @@ TEST(CreateToolExecutorTest, RegisterFla
 TEST(CreateToolExecutorTest, CreateStandaloneToolExecutor) {
   std::vector argv = {"prog", "standalone.cpp"};
   int argc = argv.size();
-  auto Executor =
-  createExecutorFromCommandLineArgs(argc, &argv[0], TestCategory);
+  auto Executor = internal::createExecutorFromCommandLineArgsImpl(
+  argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
   EXPECT_EQ(Executor->get()->getExecutorName(),
 StandaloneToolExecutor::ExecutorName);
@@ -169,8 +169,8 @@ TEST(CreateToolExecutorTest, CreateTestT
   std::vector argv = {"prog", "test.cpp",
 "--executor=test-executor"};
   int argc = argv.size();
-  auto Executor =
-  createExecutorFromCommandLineArgs(argc, &argv[0], TestCategory);
+  auto Executor = internal::createExecutorFromCommandLineArgsImpl(
+  argc, &argv[0], TestCategory);
   ASSERT_TRUE((bool)Executor);
   EXPECT_EQ(Executor->get()->getExecutorName(), 
TestToolExecutor::ExecutorName);
 }


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

[PATCH] D39676: [clangd] Add rename support.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added a subscriber: mgorny.

Make clangd handle "textDocument/rename" request. The rename
functionality comes from the "local-rename" sub-tool of clang-refactor.

Currently clangd only supports local rename (only symbol occurrences in
the main file will be renamed).


https://reviews.llvm.org/D39676

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/rename.test

Index: test/clangd/rename.test
===
--- /dev/null
+++ test/clangd/rename.test
@@ -0,0 +1,28 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+#
+Content-Length: 186
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.cpp","languageId":"cpp","version":1,"text":"int foo;"}}}
+#
+# CHECK: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.cpp","diagnostics":[]}}
+#
+
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"file:///foo.cpp"},"position":{"line":0,"character":5},"newName":"bar"}}
+#
+# CHECK: {"jsonrpc":"2.0","id":2,"result":{"changes": {"file:///foo.cpp": [{"range": {"start": {"line": 0, "character": 4}, "end": {"line": 0, "character": 7}}, "newText": "bar"}]}}}
+#
+
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -5,7 +5,7 @@
 Content-Length: 143
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 606
+# CHECK: Content-Length: 640
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
@@ -15,6 +15,7 @@
 # CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
 # CHECK:   "signatureHelpProvider": {"triggerCharacters": ["(",","]},
 # CHECK:   "definitionProvider": true,
+# CHECK:   "renameProvider": true,
 # CHECK:   "executeCommandProvider": {"commands": ["clangd.applyFix"]}
 # CHECK: }}}
 #
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -5,7 +5,7 @@
 Content-Length: 142
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":"","rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 606
+# CHECK: Content-Length: 640
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
@@ -15,6 +15,7 @@
 # CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
 # CHECK:   "signatureHelpProvider": {"triggerCharacters": ["(",","]},
 # CHECK:   "definitionProvider": true,
+# CHECK:   "renameProvider": true,
 # CHECK:   "executeCommandProvider": {"commands": ["clangd.applyFix"]}
 # CHECK: }}}
 #
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -53,6 +53,7 @@
   virtual void onSwitchSourceHeader(Ctx C, TextDocumentIdentifier &Params) = 0;
   virtual void onFileEvent(Ctx C, DidChangeWatchedFilesParams &Params) = 0;
   virtual void onCommand(Ctx C, ExecuteCommandParams &Params) = 0;
+  virtual void onRename(Ctx C, RenameParams &Parames) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,6 +71,7 @@
   Register("textDocument/definition", &ProtocolCallbacks::onGoToDefinition);
   Register("textDocument/switchSourceHeader",
&ProtocolCallbacks::onSwitchSourceHeader);
+  Register("textDocument/rename", &ProtocolCallbacks::onRename);
   Register("workspace/didChangeWatchedFiles", &ProtocolCallbacks::onFileEvent);
   Register("workspace/executeCommand",

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 121698.
xazax.hun added a comment.

- Do not warn for NonCopyable bases
- Remove lexing


https://reviews.llvm.org/D33722

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/CopyConstructorInitCheck.cpp
  clang-tidy/misc/CopyConstructorInitCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-copy-constructor-init.rst
  test/clang-tidy/misc-copy-constructor-init.cpp

Index: test/clang-tidy/misc-copy-constructor-init.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-copy-constructor-init.cpp
@@ -0,0 +1,178 @@
+// RUN: %check_clang_tidy %s misc-copy-constructor-init %t
+
+class NonCopyable {
+public:
+  NonCopyable() = default;
+  NonCopyable(const NonCopyable &) = delete;
+
+private:
+  int a;
+};
+
+class NonCopyable2 {
+public:
+  NonCopyable2() = default;
+
+private:
+  NonCopyable2(const NonCopyable2 &);
+  int a;
+};
+
+class Copyable {
+public:
+  Copyable() = default;
+  Copyable(const Copyable &) = default;
+
+private:
+  int a;
+};
+
+class Copyable2 {
+public:
+  Copyable2() = default;
+  Copyable2(const Copyable2 &) = default;
+
+private:
+  int a;
+};
+
+class Copyable3 : public Copyable {
+public:
+  Copyable3() = default;
+  Copyable3(const Copyable3 &) = default;
+};
+
+template 
+class Copyable4 {
+public:
+  Copyable4() = default;
+  Copyable4(const Copyable4 &) = default;
+
+private:
+  int a;
+};
+
+template 
+class Copyable5 {
+public:
+  Copyable5() = default;
+  Copyable5(const Copyable5 &) = default;
+
+private:
+  int a;
+};
+
+class EmptyCopyable {
+public:
+  EmptyCopyable() = default;
+  EmptyCopyable(const EmptyCopyable &) = default;
+};
+
+template 
+using CopyableAlias = Copyable5;
+
+typedef Copyable5 CopyableAlias2;
+
+class X : public Copyable, public EmptyCopyable {
+  X(const X &other) : Copyable(other) {}
+};
+
+class X2 : public Copyable2 {
+  X2(const X2 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor other than the copy constructor [misc-copy-constructor-init]
+  // CHECK-FIXES: X2(const X2 &other)  : Copyable2(other) {}
+};
+
+class X2_A : public Copyable2 {
+  X2_A(const X2_A &) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X2_A(const X2_A &) {}
+};
+
+class X3 : public Copyable, public Copyable2 {
+  X3(const X3 &other) : Copyable(other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X3(const X3 &other) : Copyable2(other), Copyable(other) {}
+};
+
+class X4 : public Copyable {
+  X4(const X4 &other) : Copyable() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X4(const X4 &other) : Copyable(other) {}
+};
+
+class X5 : public Copyable3 {
+  X5(const X5 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X5(const X5 &other)  : Copyable3(other) {}
+};
+
+class X6 : public Copyable2, public Copyable3 {
+  X6(const X6 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X6(const X6 &other)  : Copyable2(other), Copyable3(other) {}
+};
+
+class X7 : public Copyable, public Copyable2 {
+  X7(const X7 &other) : Copyable() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X7(const X7 &other) : Copyable2(other), Copyable(other) {}
+};
+
+class X8 : public Copyable4 {
+  X8(const X8 &other) : Copyable4(other) {}
+};
+
+class X9 : public Copyable4 {
+  X9(const X9 &other) : Copyable4() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X9(const X9 &other) : Copyable4(other) {}
+};
+
+class X10 : public Copyable4 {
+  X10(const X10 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X10(const X10 &other)  : Copyable4(other) {}
+};
+
+class X11 : public Copyable5 {
+  X11(const X11 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X11(const X11 &other)  : Copyable5(other) {}
+};
+
+class X12 : public CopyableAlias {
+  X12(const X12 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X12(const X12 &other) {}
+};
+
+template 
+class X13 : T {
+  X13(const X13 &other) {}
+};
+
+template class X13;
+template class X13;
+
+#define FROMMACRO\
+  class X14 : public Copyable2 { \
+X14(const X14 &other) {} \
+  };
+
+FROMMACRO
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: calling a base constructor
+
+class X15 : public CopyableAlias2 {
+  X15(const X15 &other) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling a base constructor
+  // CHECK-FIXES: X15(const X15 &other) {}
+};
+
+class X16 : public NonCopyable {
+  X16(const X16 &other) {}
+};
+
+clas

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D33722#915440, @aaron.ballman wrote:

> In the other cases, it is not clear that the re-lexed information should be 
> carried in the AST. In this case, I think it's pretty clear that the AST 
> should carry this information. Further, I don't know that the re-lexing is 
> correct (I pointed out an example that I think will be problematic) and 
> carrying the information in the AST would solve that more cleanly than trying 
> to reimplement the logic here.


Oh, you are right! Sorry for my oversight, I did not notice your example. I got 
a bit lost in all of the comments. Fortunately, I was able to get rid of the 
relexing, so this should be ok now.

>> I am not sure that we should complicate the fixit logic with the order. If 
>> -Wreorder has fixit hints, user should be able to get rid of the warning by 
>> applying that.
> 
> I disagree. We should not be changing code to be incorrect, requiring the 
> user to find that next incorrectness only through an additional compilation. 
> That is a rather poor user experience.

The resulting code is not incorrect. We do not introduce undefined behavior, 
nor change the meaning of the code. But if we do not want to introduce new 
warnings either, I could skip the fixits if there is an initializer already for 
any of the base classes. What do you think?


https://reviews.llvm.org/D33722



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

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Also, bugprone might be a better module to put this?


https://reviews.llvm.org/D33722



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

[Uriel Korach via Phabricator via cfe-commits]

uriel.k updated this revision to Diff 121699.
uriel.k marked an inline comment as done.

https://reviews.llvm.org/D38737

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/avx512vl-builtins.c
  test/CodeGen/avx512vlbw-builtins.c

Index: test/CodeGen/avx512vlbw-builtins.c
===
--- test/CodeGen/avx512vlbw-builtins.c
+++ test/CodeGen/avx512vlbw-builtins.c
@@ -2481,97 +2481,121 @@
 }
 __mmask16 test_mm_test_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
   return _mm_test_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_test_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_test_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_test_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_test_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_test_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
   return _mm_test_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_test_epi16_mask(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <8 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm256_test_epi16_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
   return _mm256_test_epi16_mask(__A, __B); 
 }
 
 __mmask16 test_mm256_mask_test_epi16_mask(__mmask16 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm_testn_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
   return _mm_testn_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_testn_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_testn_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_testn_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_testn_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_testn_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <8 x i16> %{{.*}}, %{{.*}}
   return _mm_testn_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_testn_epi16_mask(__mmask8 __U, __m128i __A, __m1

[PATCH] D39675: [clang-refactor] Use ClangTool more explicitly by making refaroing actions AST frontend actions.

[Haojian Wu via Phabricator via cfe-commits]

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

The code looks good to me.




Comment at: tools/clang-refactor/ClangRefactor.cpp:317
   RefactoringActionCommandLineOptions Options;
+  // Whether the selection is suppoted by any rule in the subcommand.
+  bool HasSelection;

s/suppoted/supported, the same above.


https://reviews.llvm.org/D39675



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

[clang-tools-extra] r317468 - [clang-tidy] Support relative paths in run-clang-tidy.py

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Nov  6 02:36:02 2017
New Revision: 317468

URL: http://llvm.org/viewvc/llvm-project?rev=317468&view=rev
Log:
[clang-tidy] Support relative paths in run-clang-tidy.py

Unfortunately, these python scripts are not tested currently. I did the testing
manually on LLVM by editing the CMake generated compilation database to
contain relative paths for some of the files. 

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=317468&r1=317467&r2=317468&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Mon Nov  6 
02:36:02 2017
@@ -68,6 +68,12 @@ def find_compilation_database(path):
   return os.path.realpath(result)
 
 
+def make_absolute(f, directory):
+  if os.path.isabs(f):
+return f
+  return os.path.normpath(os.path.join(directory, f))
+
+
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, extra_arg, extra_arg_before, quiet):
   """Gets a command line for clang-tidy."""
@@ -223,7 +229,8 @@ def main():
 
   # Load the database and extract all files.
   database = json.load(open(os.path.join(build_path, db_path)))
-  files = [entry['file'] for entry in database]
+  files = [make_absolute(entry['file'], entry['directory'])
+   for entry in database]
 
   max_task = args.j
   if max_task == 0:


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

[PATCH] D39603: [clang-tidy] Support relative paths in run-clang-tidy.py

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317468: [clang-tidy] Support relative paths in 
run-clang-tidy.py (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D39603?vs=121498&id=121705#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39603

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -68,6 +68,12 @@
   return os.path.realpath(result)
 
 
+def make_absolute(f, directory):
+  if os.path.isabs(f):
+return f
+  return os.path.normpath(os.path.join(directory, f))
+
+
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, extra_arg, extra_arg_before, quiet):
   """Gets a command line for clang-tidy."""
@@ -223,7 +229,8 @@
 
   # Load the database and extract all files.
   database = json.load(open(os.path.join(build_path, db_path)))
-  files = [entry['file'] for entry in database]
+  files = [make_absolute(entry['file'], entry['directory'])
+   for entry in database]
 
   max_task = args.j
   if max_task == 0:


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -68,6 +68,12 @@
   return os.path.realpath(result)
 
 
+def make_absolute(f, directory):
+  if os.path.isabs(f):
+return f
+  return os.path.normpath(os.path.join(directory, f))
+
+
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, extra_arg, extra_arg_before, quiet):
   """Gets a command line for clang-tidy."""
@@ -223,7 +229,8 @@
 
   # Load the database and extract all files.
   database = json.load(open(os.path.join(build_path, db_path)))
-  files = [entry['file'] for entry in database]
+  files = [make_absolute(entry['file'], entry['directory'])
+   for entry in database]
 
   max_task = args.j
   if max_task == 0:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39679: [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete created this revision.
Rakete added a project: clang.

This fixes PR34912  :)


https://reviews.llvm.org/D39679

Files:
  lib/Sema/SemaInit.cpp
  test/SemaCXX/references.cpp


Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s 
 int g(int);
 
 void f() {
@@ -55,6 +56,13 @@
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
   const int& r = cvi; // expected-error{{binding value of type 'const volatile 
int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+#if __cplusplus >= 201103L
+  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile 
int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+  const int a = 2;
+  int& r3 = a; // expected-error{{binding value of type 'const int' to 
reference to type 'int' drops 'const'}}
+#endif
 }
 
 // C++ [dcl.init.ref]p3
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7692,6 +7692,12 @@
 
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = Args[0]->getType();
+
+// For braced initializer lists, we want to get the type
+// of its (only) element, and not the "type" of the list itself.
+if (const auto *List = dyn_cast(Args[0]))
+  SourceType = List->getInit(0)->getType();
+
 QualType NonRefType = DestType.getNonReferenceType();
 Qualifiers DroppedQualifiers =
 SourceType.getQualifiers() - NonRefType.getQualifiers();


Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s 
 int g(int);
 
 void f() {
@@ -55,6 +56,13 @@
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
   const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+#if __cplusplus >= 201103L
+  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+  const int a = 2;
+  int& r3 = a; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+#endif
 }
 
 // C++ [dcl.init.ref]p3
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7692,6 +7692,12 @@
 
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = Args[0]->getType();
+
+// For braced initializer lists, we want to get the type
+// of its (only) element, and not the "type" of the list itself.
+if (const auto *List = dyn_cast(Args[0]))
+  SourceType = List->getInit(0)->getType();
+
 QualType NonRefType = DestType.getNonReferenceType();
 Qualifiers DroppedQualifiers =
 SourceType.getQualifiers() - NonRefType.getQualifiers();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39679: [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 121712.

https://reviews.llvm.org/D39679

Files:
  lib/Sema/SemaInit.cpp
  test/SemaCXX/references.cpp


Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s 
 int g(int);
 
 void f() {
@@ -55,6 +56,13 @@
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
   const int& r = cvi; // expected-error{{binding value of type 'const volatile 
int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+#if __cplusplus >= 201103L
+  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile 
int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+  const int a = 2;
+  int& r3{a}; // expected-error{{binding value of type 'const int' to 
reference to type 'int' drops 'const'}}
+#endif
 }
 
 // C++ [dcl.init.ref]p3
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7692,6 +7692,12 @@
 
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = Args[0]->getType();
+
+// For braced initializer lists, we want to get the type
+// of its (only) element, and not the "type" of the list itself.
+if (const auto *List = dyn_cast(Args[0]))
+  SourceType = List->getInit(0)->getType();
+
 QualType NonRefType = DestType.getNonReferenceType();
 Qualifiers DroppedQualifiers =
 SourceType.getQualifiers() - NonRefType.getQualifiers();


Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s 
 int g(int);
 
 void f() {
@@ -55,6 +56,13 @@
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
   const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+#if __cplusplus >= 201103L
+  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+  const int a = 2;
+  int& r3{a}; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+#endif
 }
 
 // C++ [dcl.init.ref]p3
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7692,6 +7692,12 @@
 
   case FK_ReferenceInitDropsQualifiers: {
 QualType SourceType = Args[0]->getType();
+
+// For braced initializer lists, we want to get the type
+// of its (only) element, and not the "type" of the list itself.
+if (const auto *List = dyn_cast(Args[0]))
+  SourceType = List->getInit(0)->getType();
+
 QualType NonRefType = DestType.getNonReferenceType();
 Qualifiers DroppedQualifiers =
 SourceType.getQualifiers() - NonRefType.getQualifiers();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

A breakthrough with credit going to Devin: Note that whenever we're not dealing 
with `>`/`<`/`<=`/`>=` (but only with additive ops and `==` and `!=`, and we 
have everything of the same type) we can rearrange regardless of constraints, 
simply because Z/nZ is an abelian group.

First of all, it means that we definitely do not need to check constraints in 
those cases.

Then the question is - would it be enough to limit rearrangements to additive 
and equality comparisons? `operator<()` is rarely redefined for iterator 
objects. You rely on `>`/`<` comparisons to compare symbolic iterators to 
`.begin()` or `.end()`, but how much more does it give in practice, compared to 
straightforwardly matching, say, `i == i.container.end` where `N` is 
non-negative, given that the rest of our solver barely solves anything? Like, 
let's say that "iterator `i` is past end" from now on literally means "//there 
exists a non-negative `N` for which `i` is-the-same-symbol-as `i.container.end 
+ N`//" rather than "//`i >= i.container.end`//". Would the checker lose much 
from such change? If not, we can completely screw all checks and gain a lot 
more value from the rearrangements.


https://reviews.llvm.org/D35109



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

[Uriel Korach via Phabricator via cfe-commits]

uriel.k updated this revision to Diff 121716.

https://reviews.llvm.org/D38737

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/avx512vl-builtins.c
  test/CodeGen/avx512vlbw-builtins.c

Index: test/CodeGen/avx512vlbw-builtins.c
===
--- test/CodeGen/avx512vlbw-builtins.c
+++ test/CodeGen/avx512vlbw-builtins.c
@@ -2481,97 +2481,121 @@
 }
 __mmask16 test_mm_test_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
   return _mm_test_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_test_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_test_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_test_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_test_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_test_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
   return _mm_test_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_test_epi16_mask(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <8 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <8 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm256_test_epi16_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
   return _mm256_test_epi16_mask(__A, __B); 
 }
 
 __mmask16 test_mm256_mask_test_epi16_mask(__mmask16 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_test_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestm.w.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp ne <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_test_epi16_mask(__U, __A, __B); 
 }
 
 __mmask16 test_mm_testn_epi8_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
   return _mm_testn_epi8_mask(__A, __B); 
 }
 
 __mmask16 test_mm_mask_testn_epi8_mask(__mmask16 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask32 test_mm256_testn_epi8_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
   return _mm256_testn_epi8_mask(__A, __B); 
 }
 
 __mmask32 test_mm256_mask_testn_epi8_mask(__mmask32 __U, __m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_testn_epi8_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.b.256
+  // CHECK: and <4 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_testn_epi8_mask(__U, __A, __B); 
 }
 
 __mmask8 test_mm_testn_epi16_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_testn_epi16_mask
-  // CHECK: @llvm.x86.avx512.ptestnm.w.128
+  // CHECK: and <2 x i64> %{{.*}}, %{{.*}}
+  // CHECK: icmp eq <8 x i16> %{{.*}}, %{{.*}}
   return _mm_testn_epi16_mask(__A, __B); 
 }
 
 __mmask8 test_mm_mask_testn_epi16_mask(__mmask8 __U, __m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_mas

r317473 - [clang-format] Handle unary operator overload with arguments and specifiers

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Nov  6 04:11:51 2017
New Revision: 317473

URL: http://llvm.org/viewvc/llvm-project?rev=317473&view=rev
Log:
[clang-format] Handle unary operator overload with arguments and specifiers

Before:
  int operator++(int)noexcept;

After:
  int operator++(int) noexcept;

Patch by Igor Sugak. Thank you!

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=317473&r1=317472&r2=317473&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Nov  6 04:11:51 2017
@@ -696,7 +696,8 @@ private:
   CurrentToken->Type = TT_PointerOrReference;
 consumeToken();
 if (CurrentToken &&
-CurrentToken->Previous->isOneOf(TT_BinaryOperator, tok::comma))
+CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
+tok::comma))
   CurrentToken->Previous->Type = TT_OverloadedOperator;
   }
   if (CurrentToken) {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=317473&r1=317472&r2=317473&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Nov  6 04:11:51 2017
@@ -5591,6 +5591,7 @@ TEST_F(FormatTest, UnderstandsOverloaded
   verifyFormat("bool operator!=();");
   verifyFormat("int operator+();");
   verifyFormat("int operator++();");
+  verifyFormat("int operator++(int) volatile noexcept;");
   verifyFormat("bool operator,();");
   verifyFormat("bool operator();");
   verifyFormat("bool operator()();");


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

[PATCH] D39587: [clang-format] Handle unary operator overload with arguments and specifiers

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Submitted as r317473. Thank you!


https://reviews.llvm.org/D39587



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Thank you for your comment!

Unfortunately, the iterator checkers depend very much on the >/>=/=C.end(), I cannot imagine this 
to be solved just with ==/!=. How to find the N if we only use == or !=? 
Furthermore, later parts also add invalidation check where we must find 
iterator positions which are in some relation with the invalidated position. 
For example, insertion into a vector invalidates every i>=cur positions.


https://reviews.llvm.org/D35109



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

[PATCH] D39682: [analyzer] Fix a crash on logical operators with vectors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

We crash whenever we try to compute `x && y` or `x || y` where `x` and `y` are 
of vector type.

For now we do not seem to properly model operations with vectors. In 
particular, operations `&&` and `||` on two values of type `int 
__attribute__((ext_vector_type(2)))` are not short-circuit, unlike regular 
logical operators, so even our CFG is incorrect.

Avoid the crash, add respective FIXME tests for later.


https://reviews.llvm.org/D39682

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/vector.c


Index: test/Analysis/vector.c
===
--- /dev/null
+++ test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify 
%s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V no_crash(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -625,6 +625,17 @@
   assert(B->getOpcode() == BO_LAnd ||
  B->getOpcode() == BO_LOr);
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, Pred->getState());
+return;
+  }
+
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 


Index: test/Analysis/vector.c
===
--- /dev/null
+++ test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V no_crash(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -625,6 +625,17 @@
   assert(B->getOpcode() == BO_LAnd ||
  B->getOpcode() == BO_LOr);
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, Pred->getState());
+return;
+  }
+
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> How to find the N if we only use == or !=?

Hence the difference between `==` and `is-the-same-symbol-as`. We can find `N` 
by looking at the symbol.

We'd lose track of cases where, say, `i` and `.end()` were compared to each 
other for `>`/`<` before. The question is about how common such cases are.


https://reviews.llvm.org/D35109



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

[PATCH] D39682: [analyzer] Fix a crash on logical operators with vectors.

[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.

LGTM!




Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:633
+// modeled as short-circuit in Clang CFG but this is incorrect.
+StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+// Do not set the value for the expression. It'd be UnknownVal by default.

I was wondering maybe moving this code down a bit you could reuse the 
`StmtNodeBuilder` bellow and the could be slightly shorter. But I guess it is 
just the matter of taste. 


https://reviews.llvm.org/D39682



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

[PATCH] D39682: [analyzer] Fix a crash on logical operators with vectors.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 121725.
NoQ added a comment.

Yep, right!


https://reviews.llvm.org/D39682

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/vector.c


Index: test/Analysis/vector.c
===
--- /dev/null
+++ test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify 
%s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V no_crash(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -628,6 +628,16 @@
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, state);
+return;
+  }
+
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs()) {
 ProgramPoint P = N->getLocation();


Index: test/Analysis/vector.c
===
--- /dev/null
+++ test/Analysis/vector.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V no_crash(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -628,6 +628,16 @@
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
+  if (B->getType()->isVectorType()) {
+// FIXME: We do not model vector arithmetic yet. When adding support for
+// that, note that the CFG-based reasoning below does not apply, because
+// logical operators on vectors are not short-circuit. Currently they are
+// modeled as short-circuit in Clang CFG but this is incorrect.
+// Do not set the value for the expression. It'd be UnknownVal by default.
+Bldr.generateNode(B, Pred, state);
+return;
+  }
+
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs()) {
 ProgramPoint P = N->getLocation();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 121726.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.

I have updated the patch so it uses evalBinOpNN. This seems to work properly.

I have a number of TODOs in the test cases that should be fixed. Truncations 
are not handled properly.

Here is a short example code:

  void f(unsigned char X) {
if (X >= 10 && X <= 50) {
  unsigned char Y = X + 0x100; // truncation
  clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}}
}
  }

The expected-warning should be TRUE but currently FALSE is written.

When the "Y >= 10" condition is evaluated the ProgramState is:

  Store (direct and default bindings), 0x222ab0fe5f8 :
   (Y,0,direct) : (unsigned char) ((reg_$0) + 256)
  
  Expressions:
   (0x222a96d6050,0x222ab0eb930) X + 256 : (unsigned char) ((reg_$0) + 256)
   (0x222a96d6050,0x222ab0eb960) clang_analyzer_eval : 
&code{clang_analyzer_eval}
   (0x222a96d6050,0x222ab0eb988) Y : &Y
   (0x222a96d6050,0x222ab0eb9d8) Y : (unsigned char) ((reg_$0) 
+ 256)
   (0x222a96d6050,0x222ab0eb9f0) Y : (unsigned char) ((reg_$0) 
+ 256)
   (0x222a96d6050,0x222ab0eba08) Y >= 10 : ((unsigned char) ((reg_$0) + 256)) >= 10
   (0x222a96d6050,0x222ab0ebb28) clang_analyzer_eval : 
&code{clang_analyzer_eval}
  Ranges of symbol values:
   reg_$0 : { [10, 50] }
   (reg_$0) + 256 : { [10, 50] }

It seems to me that the symbol initialization does not handle the range 
properly. Imho there is nothing wrong with the calculation. What you think 
about adding a range like below?

  (unsigned char) ((reg_$0) + 256) : { [10, 50] }




Repository:
  rL LLVM

https://reviews.llvm.org/D36471

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/range_calc.c

Index: test/Analysis/range_calc.c
===
--- test/Analysis/range_calc.c
+++ test/Analysis/range_calc.c
@@ -0,0 +1,143 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+#define INT_MAX((signed int)((~0U)>>1))
+#define INT_MIN((signed int)(~((~0U)>>1)))
+
+void addInts(int X)
+{
+  if (X >= 10 && X <= 50) {
+int Y = X + 2;
+clang_analyzer_eval(Y >= 12 && Y <= 52); // expected-warning{{TRUE}}
+  }
+  
+  if (X < 5) {
+int Y = X + 1;
+clang_analyzer_eval(Y < 6); // expected-warning{{TRUE}}
+  }
+
+  if (X >= 1000) {
+int Y = X + 1; // might overflow
+clang_analyzer_eval(Y >= 1001); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(Y == INT_MIN); // expected-warning{{UNKNOWN}}
+	clang_analyzer_eval(Y == INT_MIN || Y >= 1001); // expected-warning{{TRUE}}
+  }
+}
+
+void addU8(unsigned char X)
+{
+  if (X >= 10 && X <= 50) {
+unsigned char Y = X + 2;
+clang_analyzer_eval(Y >= 12 && Y <= 52); // expected-warning{{TRUE}}
+  }
+  
+  if (X < 5) {
+unsigned char Y = X + 1;
+clang_analyzer_eval(Y < 6); // expected-warning{{TRUE}}
+  }
+
+  // TODO
+  if (X >= 10 && X <= 50) {
+unsigned char Y = X + (-256); // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}}
+  }
+
+  // TODO
+  if (X >= 10 && X <= 50) {
+unsigned char Y = X + 256; // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}} expected-warning{{UNKNOWN}}
+  }
+
+  // TODO
+  if (X >= 100) {
+unsigned char Y = X + 1; // truncation
+	clang_analyzer_eval(Y == 0); // expected-warning{{FALSE
+	clang_analyzer_eval(Y >= 101); // expected-warning{{TRUE}}
+clang_analyzer_eval(Y == 0 || Y >= 101); // expected-warning{{TRUE}}
+  }
+
+  if (X >= 100) {
+unsigned short Y = X + 1;
+clang_analyzer_eval(Y >= 101 && Y <= 256); // expected-warning{{TRUE}}
+  }
+}
+
+void sub1(int X)
+{
+  if (X >= 10 && X <= 50) {
+int Y = X - 2;
+clang_analyzer_eval(Y >= 8 && Y <= 48); // expected-warning{{TRUE}}
+  }
+
+  if (X >= 10 && X <= 50) {
+unsigned char Y = (unsigned int)X - 20; // truncation
+clang_analyzer_eval(Y <= 30 || Y >= 246); // expected-warning{{TRUE}}
+  }
+
+  // TODO
+  if (X >= 10 && X <= 50) {
+unsigned char Y = (unsigned int)X - 256; // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}} expected-warning{{UNKNOWN}}
+  }
+
+  if (X < 5) {
+int Y = X - 1; // might overflow
+clang_analyzer_eval(Y < 4); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(Y == INT_MAX); // expected-warning{{UNKNOWN}}
+	clang_analyzer_eval(Y == INT_MAX || Y < 4); // expected-warning{{TRUE}}
+  }
+
+  if (X >= 1000) {
+int Y = X - 1;
+clang_analyzer_eval(Y >= 999); // expected-warning{{TRUE}}
+  }
+}
+
+void subU8(unsigned char X)
+{
+  if (X >= 10 && X <= 50) {
+unsigned char Y = X - 2;
+clang_analyzer_eval(Y >= 

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35109#916648, @NoQ wrote:

> > How to find the N if we only use == or !=?
>
> Hence the difference between `==` and `is-the-same-symbol-as`. We can find 
> `N` by looking at the symbol.


Sorry, if I misunderstand something, but if I do not know the `N` and have to 
look it up then I have to create a difference manually (as I did it originally) 
to look it up.

> We'd lose track of cases where, say, `i` and `.end()` were compared to each 
> other for `>`/`<` before. The question is about how common such cases are.

`i` and `.begin()` are compared to each other for `<`. And (as I mentioned) 
there will be lots of `<`, `<=`, `>` and `<=` operators when invalidating.


https://reviews.llvm.org/D35109



___
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

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 12 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c:1
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+

aaron.ballman wrote:
> Please clang-format this file.
I omitted it intantionally. No I did it, but it meant lots of editing back the 
`CHECK-MESSAGES` and `CHECK-FIXES` comments. They were broken were they should 
not have been, and merged where they should not have been.



Comment at: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp:1
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+

aaron.ballman wrote:
> Please clang-format this file.
Same as above.


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] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D35109#916617, @NoQ wrote:

> A breakthrough with credit going to Devin: Note that whenever we're not 
> dealing with `>`/`<`/`<=`/`>=` (but only with additive ops and `==` and `!=`, 
> and we have everything of the same type) we can rearrange regardless of 
> constraints, simply because Z/nZ is an abelian group.
>
> First of all, it means that we definitely do not need to check constraints in 
> those cases.


Does this mean it might be efficient enough that we want to implement this 
regardless what will be the final solution for the iterator checkers? Or we 
need a strong use case to accept such a change into core?


https://reviews.llvm.org/D35109



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

[PATCH] D39505: [OpenMP] Show error if VLAs are not supported

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld marked an inline comment as done.
Hahnfeld added a subscriber: gtbercea.
Hahnfeld added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:944
+  /// \brief Whether target supports variable-length arrays.
+  bool isVLASupported() const { return VLASupported; }
+

rjmccall wrote:
> Hahnfeld wrote:
> > rjmccall wrote:
> > > Hahnfeld wrote:
> > > > rjmccall wrote:
> > > > > The way you've written this makes it sound like "does the target 
> > > > > support VLAs?", but the actual semantic checks treat it as "do OpenMP 
> > > > > devices on this target support VLAs?"  Maybe there should be a more 
> > > > > specific way to query things about OpenMP devices instead of setting 
> > > > > a global flag for the target?
> > > > Actually, the NVPTX and SPIR targets never support VLAs. So I felt like 
> > > > it would be more correct to make this a global property of the target.
> > > > 
> > > > The difference is that the other programming models (OpenCL and CUDA) 
> > > > error out immediatelyand regardless of the target because this 
> > > > limitation is reflected in the standards that disallow VLAs (see 
> > > > SemaType.cpp). For OpenMP we might have target devices that support VLA 
> > > > so we shouldn't error out for those.
> > > If you want to make it a global property of the target, that's fine, but 
> > > then I don't understand why your diagnostic only fires when 
> > > (S.isInOpenMPDeclareTargetContext() || 
> > > S.isInOpenMPTargetExecutionDirective()).
> > That is because of how OpenMP offloading works and how it is implemented in 
> > Clang. Consider the following snippet from the added test case:
> > ```lang=c
> > int vla[arg];
> > 
> > #pragma omp target map(vla[0:arg])
> > {
> >// more code here...
> > }
> > ```
> > 
> > Clang will take the following steps to compile this into a working binary 
> > for a GPU:
> > 1. Parse and (semantically) analyze the code as-is for the host and produce 
> > LLVM Bitcode.
> > 2. Parse and analyze again the code as-is and generate code for the 
> > offloading target, the GPU in this case.
> > 3. Take LLVM Bitcode from 1., generate host binary and embed target binary 
> > from 3.
> > 
> > `OpenMPIsDevice` will be true for 2., but the complete source code is 
> > analyzed. So to not throw errors for the host code, we have to make sure 
> > that we are actually generating code for the target device. This is either 
> > in a `target` directive or in a `declare target` region.
> > Note that this is quite similar to what CUDA does, only they have 
> > `CUDADiagIfDeviceCode` for this logic. If you want me to add something of 
> > that kind for OpenMP target devices, I'm fine with that. However for the 
> > given case, it's a bit different because this error should only be thrown 
> > for target devices that don't support VLAs...
> I see.  So the entire translation unit is re-parsed and re-Sema'ed from 
> scratch for the target?  Which means you need to avoid generating errors 
> about things in the outer translation unit that aren't part of the target 
> directive that you actually want to compile.  I would've expected there to be 
> some existing mechanism for that, to be honest, as opposed to explicitly 
> trying to suppress target-specific diagnostics one by one.
Yes, that is my understanding. For errors, we don't need to take anything 
special as the first `cc1` invocation will exit with a non-zero status so that 
the driver stops the compilation. For warnings, there seems to be no mechanism 
in place as I see them duplicated, even in code that is not generate for the 
target device (verified with an unused variable).

@ABataev @gtbercea Do I miss something here?


https://reviews.llvm.org/D39505



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

[PATCH] D39505: [OpenMP] Show error if VLAs are not supported

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:944
+  /// \brief Whether target supports variable-length arrays.
+  bool isVLASupported() const { return VLASupported; }
+

Hahnfeld wrote:
> rjmccall wrote:
> > Hahnfeld wrote:
> > > rjmccall wrote:
> > > > Hahnfeld wrote:
> > > > > rjmccall wrote:
> > > > > > The way you've written this makes it sound like "does the target 
> > > > > > support VLAs?", but the actual semantic checks treat it as "do 
> > > > > > OpenMP devices on this target support VLAs?"  Maybe there should be 
> > > > > > a more specific way to query things about OpenMP devices instead of 
> > > > > > setting a global flag for the target?
> > > > > Actually, the NVPTX and SPIR targets never support VLAs. So I felt 
> > > > > like it would be more correct to make this a global property of the 
> > > > > target.
> > > > > 
> > > > > The difference is that the other programming models (OpenCL and CUDA) 
> > > > > error out immediatelyand regardless of the target because this 
> > > > > limitation is reflected in the standards that disallow VLAs (see 
> > > > > SemaType.cpp). For OpenMP we might have target devices that support 
> > > > > VLA so we shouldn't error out for those.
> > > > If you want to make it a global property of the target, that's fine, 
> > > > but then I don't understand why your diagnostic only fires when 
> > > > (S.isInOpenMPDeclareTargetContext() || 
> > > > S.isInOpenMPTargetExecutionDirective()).
> > > That is because of how OpenMP offloading works and how it is implemented 
> > > in Clang. Consider the following snippet from the added test case:
> > > ```lang=c
> > > int vla[arg];
> > > 
> > > #pragma omp target map(vla[0:arg])
> > > {
> > >// more code here...
> > > }
> > > ```
> > > 
> > > Clang will take the following steps to compile this into a working binary 
> > > for a GPU:
> > > 1. Parse and (semantically) analyze the code as-is for the host and 
> > > produce LLVM Bitcode.
> > > 2. Parse and analyze again the code as-is and generate code for the 
> > > offloading target, the GPU in this case.
> > > 3. Take LLVM Bitcode from 1., generate host binary and embed target 
> > > binary from 3.
> > > 
> > > `OpenMPIsDevice` will be true for 2., but the complete source code is 
> > > analyzed. So to not throw errors for the host code, we have to make sure 
> > > that we are actually generating code for the target device. This is 
> > > either in a `target` directive or in a `declare target` region.
> > > Note that this is quite similar to what CUDA does, only they have 
> > > `CUDADiagIfDeviceCode` for this logic. If you want me to add something of 
> > > that kind for OpenMP target devices, I'm fine with that. However for the 
> > > given case, it's a bit different because this error should only be thrown 
> > > for target devices that don't support VLAs...
> > I see.  So the entire translation unit is re-parsed and re-Sema'ed from 
> > scratch for the target?  Which means you need to avoid generating errors 
> > about things in the outer translation unit that aren't part of the target 
> > directive that you actually want to compile.  I would've expected there to 
> > be some existing mechanism for that, to be honest, as opposed to explicitly 
> > trying to suppress target-specific diagnostics one by one.
> Yes, that is my understanding. For errors, we don't need to take anything 
> special as the first `cc1` invocation will exit with a non-zero status so 
> that the driver stops the compilation. For warnings, there seems to be no 
> mechanism in place as I see them duplicated, even in code that is not 
> generate for the target device (verified with an unused variable).
> 
> @ABataev @gtbercea Do I miss something here?
I'm not aware of any.


https://reviews.llvm.org/D39505



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 121700.
xazax.hun added a comment.

- Rebased on ToT


https://reviews.llvm.org/D30691

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/CMakeLists.txt
  test/Analysis/Inputs/ctu-chain.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/libscanbuild/report.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -92,3 +92,16 @@
 self.assertEqual('Checker One description', result.get('checker.one'))
 self.assertTrue('checker.two' in result)
 self.assertEqual('Checker Two description', result.get('checker.two'))
+
+
+class ClangIsCtuCapableTest(unittest.TestCase):
+def test_ctu_not_found(self):
+is_ctu = sut.is_ctu_capable('not-found-clang',
+'not-found-clang-func-mapping')
+self.assertFalse(is_ctu)
+
+
+class ClangGetTripleArchTest(unittest.TestCase):
+def test_arch_is_not_empty(self):
+arch = sut.get_triple_arch(['clang', '-E', '-'], '.')
+self.assertTrue(len(arch) > 0)
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -4,12 +4,12 @@
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
-import libear
-import libscanbuild.analyze as sut
 import unittest
 import re
 import os
 import os.path
+import libear
+import libscanbuild.analyze as sut
 
 
 class ReportDirectoryTest(unittest.TestCase):
@@ -333,3 +333,83 @@
 
 def test_method_exception_not_caught(self):
 self.assertRaises(Exception, method_exception_from_inside, dict())
+
+
+class PrefixWithTest(unittest.TestCase):
+
+def test_gives_empty_on_empty(self):
+res = sut.prefix_with(0, [])
+self.assertFalse(res)
+
+def test_interleaves_prefix(self):
+res = sut.prefix_with(0, [1, 2, 3])
+self.assertListEqual([0, 1, 0, 2, 0, 3], res)
+
+
+class MergeCtuMapTest(unittest.TestCase):
+
+def test_no_map_gives_empty(self):
+pairs = sut.create_global_ctu_function_map([])
+self.assertFalse(pairs)
+
+def test_multiple_maps_merged(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun3#I# ast/fun3.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
+self.assertEqual(3, len(pairs))
+
+def test_not_unique_func_left_out(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun7.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+def test_duplicates_are_kept(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun1.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(2, len(pairs))
+
+def test_space_handled_in_source(self):
+concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+
+class FuncMapSrcToAstTest(unittest.TestCase):
+
+def test_empty_gives_empty(self):
+fun_ast_l

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

[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!




Comment at: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.c:1
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+

baloghadamsoftware wrote:
> aaron.ballman wrote:
> > Please clang-format this file.
> I omitted it intantionally. No I did it, but it meant lots of editing back 
> the `CHECK-MESSAGES` and `CHECK-FIXES` comments. They were broken were they 
> should not have been, and merged where they should not have been.
Ugh, it's unfortunate that clang-format messed with the CHECK-* stuff, but 
thank you for ensuring the code is formatted according to the community 
standards.


https://reviews.llvm.org/D39121



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

[clang-tools-extra] r317487 - [clangd] Squash namespace warning

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Nov  6 07:50:35 2017
New Revision: 317487

URL: http://llvm.org/viewvc/llvm-project?rev=317487&view=rev
Log:
[clangd] Squash namespace warning

Modified:
clang-tools-extra/trunk/clangd/JSONExpr.cpp

Modified: clang-tools-extra/trunk/clangd/JSONExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONExpr.cpp?rev=317487&r1=317486&r2=317487&view=diff
==
--- clang-tools-extra/trunk/clangd/JSONExpr.cpp (original)
+++ clang-tools-extra/trunk/clangd/JSONExpr.cpp Mon Nov  6 07:50:35 2017
@@ -180,11 +180,16 @@ void clang::clangd::json::Expr::print(ra
   }
 }
 
-llvm::raw_ostream &clang::clangd::json::operator<<(raw_ostream &OS,
-   const Expr &E) {
+namespace clang {
+namespace clangd {
+namespace json {
+llvm::raw_ostream &operator<<(raw_ostream &OS, const Expr &E) {
   E.print(OS, [](IndenterAction A) { /*ignore*/ });
   return OS;
 }
+} // namespace json
+} // namespace clangd
+} // namespace clang
 
 void llvm::format_provider::format(
 const clang::clangd::json::Expr &E, raw_ostream &OS, StringRef Options) {


___
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

[Barancsuk Lilla via Phabricator via cfe-commits]

barancsuk updated this revision to Diff 121740.
barancsuk marked 2 inline comments as done.
barancsuk added a comment.

Address review comments: fix missing full stops at the end of comments


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" == + 

r317489 - [CodeGen] match new fast-math-flag method: isFast()

[Sanjay Patel via cfe-commits]

Author: spatel
Date: Mon Nov  6 08:27:36 2017
New Revision: 317489

URL: http://llvm.org/viewvc/llvm-project?rev=317489&view=rev
Log:
[CodeGen] match new fast-math-flag method: isFast()

This corresponds to LLVM commiti r317488:

If that commit is reverted, this commit will also need to be reverted.

Modified:
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=317489&r1=317488&r2=317489&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Mon Nov  6 08:27:36 2017
@@ -87,7 +87,7 @@ CodeGenFunction::CodeGenFunction(CodeGen
 
   llvm::FastMathFlags FMF;
   if (CGM.getLangOpts().FastMath)
-FMF.setUnsafeAlgebra();
+FMF.setFast();
   if (CGM.getLangOpts().FiniteMathOnly) {
 FMF.setNoNaNs();
 FMF.setNoInfs();


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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In https://reviews.llvm.org/D39502#916409, @jlebar wrote:

> > Note the host clang side has the inputs "test.cu" and "test-c3378c.fatbin". 
> > Which means if the device side compilation failed, the host side will not 
> > compile because InputOk will return false in this case
>
> The patch handles this case correctly, thank you.
>
> Unfortunately it's a bit more complicated than this.  You can pass 
> `--cuda-gpu-arch=XXX` multiple times when invoking clang, and that will 
> result in multiple compilations for different GPU architectures.  In this 
> case we want to fail when the first one fails, on the theory that (most of 
> the time) all compilations will have the same errors.
>
> You can test this with e.g.
>
>   $ clang -x cuda test.cu -nocudalib -nocudainc --cuda-gpu-arch=sm_35 
> --cuda-gpu-arch=sm_60
>
> With this patch, clang prints the errors twice, once for sm_35 and once for 
> sm_60.  Without the patch, it only prints one set of errors, which is the 
> expected behavior.


Ok. I didn't thought about that. To fix that, I guess the driver need to 
invalidate all the offload input once one compilation failed. Let me see how to 
do that. Thanks for looking.

> 
> 
>> I don't know what is the best way to write test cases for CUDA driver.
> 
> Thanks for offering to write tests for this.  See `ls clang/test/Driver | 
> grep '\.cu$'`, and let me know if you need help crafting a testcase.


https://reviews.llvm.org/D39502



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

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 121743.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

In https://reviews.llvm.org/D39462#912011, @rjmccall wrote:

> The actual choice of integer type is not stable across targets any more than 
> the size is.  In practice, people often don't use int and long, they use 
> standard typedefs like size_t and uint64_t, but the actual type chosen for 
> size_t is arbitrary when there are multiple types at that bit-width.




In https://reviews.llvm.org/D39462#912387, @efriedma wrote:

> > The internal canonical types are compared, so all this typedef sugar will 
> > be silently ignored.
>
> Yes, and that's precisely the problem.  On many 32-bit platforms, both 
> "size_t" and "uint32_t" are typedefs for "unsigned int"; on some 32-bit 
> platforms, "size_t" is an "unsigned long".  So whether this patch suppresses 
> the warning for a comparison between size_t and uint32_t depends on the 
> target ABI.


Okay, changed the code to compare not the internal canonical types, but just 
the types.
Added tests that show that it does detect&complain about comparing two 
different types, that point to the same type.

Also. should it really not be in `-Wextra`?


Repository:
  rL LLVM

https://reviews.llvm.org/D39462

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/maybe-tautological-constant-compare.c
  test/Sema/maybe-tautological-constant-compare.cpp

Index: test/Sema/maybe-tautological-constant-compare.cpp
===
--- /dev/null
+++ test/Sema/maybe-tautological-constant-compare.cpp
@@ -0,0 +1,194 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -Wmaybe-tautological-constant-compare -DTEST -std=c++11  -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DLP64 -std=c++11 -verify %s
+// RUN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -Wmaybe-tautological-constant-compare -DTEST -std=c++11 -verify %s
+// RUN: %clang_cc1 -triple i386-linux-gnu -fsyntax-only -DILP32 -std=c++11 -verify %s
+
+template  struct numeric_limits {
+  static constexpr T max() noexcept { return T(~0); }
+};
+
+int value(void);
+
+int main() {
+  unsigned long ul = value();
+
+  using T1 = unsigned int;
+  T1 t1 = value();
+  using T2 = unsigned int;
+
+#if defined(LP64)
+#if __SIZEOF_INT__ != 4
+#error Expecting 32-bit int
+#endif
+#if __SIZEOF_LONG__ != 8
+#error Expecting 64-bit int
+#endif
+#elif defined(ILP32)
+#if __SIZEOF_INT__ != 4
+#error Expecting 32-bit int
+#endif
+#if __SIZEOF_LONG__ != 4
+#error Expecting 32-bit int
+#endif
+#else
+#error LP64 or ILP32 should be defined
+#endif
+
+#if defined(TEST)
+  if (t1 < numeric_limits::max())
+return 0;
+  if (numeric_limits::max() >= t1) // expected-warning {{for the current target platform, comparison 4294967295 >= 'T1' (aka 'unsigned int') is always true}}
+return 0;
+  if (t1 > numeric_limits::max()) // expected-warning {{for the current target platform, comparison 'T1' (aka 'unsigned int') > 4294967295 is always false}}
+return 0;
+  if (numeric_limits::max() <= t1)
+return 0;
+  if (t1 <= numeric_limits::max()) // expected-warning {{for the current target platform, comparison 'T1' (aka 'unsigned int') <= 4294967295 is always true}}
+return 0;
+  if (numeric_limits::max() > t1)
+return 0;
+  if (t1 >= numeric_limits::max())
+return 0;
+  if (numeric_limits::max() < t1) // expected-warning {{for the current target platform, comparison 4294967295 < 'T1' (aka 'unsigned int') is always false}}
+return 0;
+  if (t1 == numeric_limits::max())
+return 0;
+  if (numeric_limits::max() != t1)
+return 0;
+  if (t1 != numeric_limits::max())
+return 0;
+  if (numeric_limits::max() == t1)
+return 0;
+
+  if (t1 < numeric_limits::max())
+return 0;
+  if (numeric_limits::max() >= t1) // expected-warning {{for the current target platform, comparison 4294967295 >= 'T1' (aka 'unsigned int') is always true}}
+return 0;
+  if (t1 > numeric_limits::max()) // expected-warning {{for the current target platform, comparison 'T1' (aka 'unsigned int') > 4294967295 is always false}}
+return 0;
+  if (numeric_limits::max() <= t1)
+return 0;
+  if (t1 <= numeric_limits::max()) // expected-warning {{for the current target platform, comparison 'T1' (aka 'unsigned int') <= 4294967295 is always true}}
+return 0;
+  if (numeric_limits::max() > t1)
+return 0;
+  if (t1 >= numeric_limits::max())
+return 0;
+  if (numeric_limits::max() < t1) // expected-warning {{for the current target platform, comparison 4294967295 < 'T1' (aka 'unsigned int') is always false}}
+return 0;
+  if (t1 == numeric_limits::max())
+return 0;
+  if (numeric_limits::max() != t1)
+return 0;
+  if (t1 != numeric_limits::max())
+return 0;
+  if (numeric_limits::max() == t1)
+ret

[PATCH] D39332: [clang-refactor] Introduce a new rename rule for qualified symbols

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



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

hokein wrote:
> arphaman wrote:
> > ioeric wrote:
> > > hokein wrote:
> > > > arphaman wrote:
> > > > > 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());
> > > > > ```
> > > > Done. Introduced a common interface `USRRenameRule`.
> > > `USRRenameRule` doesn't seem to be a very useful abstraction. I think 
> > > what Alex proposed is a base class that implements 
> > > `createSourceReplacements` which could be shared by both 
> > > `RenameOccurrences` and `QualifiedRenameRule`. Intuitively, un-qualified 
> > > rename is a special case of qualified rename (maybe?), where namespace is 
> > > not changed.
> > Yep, I meant that indeed.
> Ah, sorry for the misunderstanding. 
> 
> Unfortunately, `RenameOccurrences` and `QualifiedRenameRule` could not share 
> the same `createSourceReplacements` implementation, 
> 
> * Their supported use cases are different. `QualifiedRenameRule` only 
> supports renaming class, function, enums, alias and class members, which 
> doesn't cover all the cases of `RenameOccurrences` (like renaming local 
> variable in function body).
> 
> * we have separated implementations in the code 
> base(`createRenameAtomicChanges` for qualified rename, 
> `createRenameReplacements` for un-qualified rename). The implementation of 
> qualified rename does more things, including calculating the shortest prefix 
> qualifiers, getting correct range of replacement.  
> 
> 
That makes sense (I think), but then we should remove  `USRRenameRule` again if 
we're not actually reusing anything.

(And ideally we can hide any such future reuse as functions in the cc file, 
rather than a class hierarchy exposed in the header)


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] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D33722#916539, @xazax.hun wrote:

> In https://reviews.llvm.org/D33722#915440, @aaron.ballman wrote:
>
> > I disagree. We should not be changing code to be incorrect, requiring the 
> > user to find that next incorrectness only through an additional 
> > compilation. That is a rather poor user experience.
>
>
> The resulting code is not incorrect. We do not introduce undefined behavior.


Undefined behavior is not the only form of incorrectness. The resulting code is 
incorrect because the initalization order does not match the declaration order 
and relying on that behavior leads to bugs, which is why -Wreorder is on by 
default.

> But if we do not want to introduce new warnings either, I could skip the 
> fixits if there is an initializer already for any of the base classes. What 
> do you think?

I think that's reasonable behavior.




Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:38-40
+  if (!Class->field_empty())
+return false;
+  return true;

I'm not certain this function is all that useful -- it could be replaced with 
`Class->field_empty()` in the caller (even as a lambda if needed).



Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:69
+  (Ctor->getAccess() == AS_private || Ctor->isDeleted())) {
+NonCopyableBase = true;
+break;

What if the base class is inherited privately? e.g.,
```
struct Base {
  Base(const Base&) {}
};

struct Derived : private Base {
  Derived(const Derived &) {}
};
```



Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:109
+  } else {
+// We apply the missing ctros at the beginning of the initialization list.
+FixItLoc = (*Ctor->init_begin())->getSourceLocation();

ctros -> ctors



Comment at: docs/clang-tidy/checks/misc-copy-constructor-init.rst:4
+misc-copy-constructor-init
+=
+

The markdown here is wrong.



Comment at: docs/clang-tidy/checks/misc-copy-constructor-init.rst:6
+
+Finds copy constructors where the constructor don't call 
+the constructor of the base class.

don't -> doesn't


https://reviews.llvm.org/D33722



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

r317493 - [Parser] Fix TryParseLambdaIntroducer() error handling

[Jan Korous via cfe-commits]

Author: jkorous
Date: Mon Nov  6 09:42:17 2017
New Revision: 317493

URL: http://llvm.org/viewvc/llvm-project?rev=317493&view=rev
Log:
[Parser] Fix TryParseLambdaIntroducer() error handling

rdar://35066196

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

Added:
cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
Modified:
cfe/trunk/lib/Parse/ParseExprCXX.cpp

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=317493&r1=317492&r2=317493&view=diff
==
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Mon Nov  6 09:42:17 2017
@@ -991,27 +991,34 @@ Optional Parser::ParseLambdaIn
 ///
 /// 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

Added: cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp?rev=317493&view=auto
==
--- cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp (added)
+++ cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp Mon Nov  6 09:42:17 2017
@@ -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 '}'}}


___
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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317493: [Parser] Fix TryParseLambdaIntroducer() error 
handling (authored by jkorous).

Changed prior to commit:
  https://reviews.llvm.org/D39419?vs=121448&id=121748#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39419

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


Index: cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
===
--- cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
+++ cfe/trunk/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 '}'}}
Index: cfe/trunk/lib/Parse/ParseExprCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp
+++ cfe/trunk/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: cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
===
--- cfe/trunk/test/Parser/objcxx11-invalid-lambda.cpp
+++ cfe/trunk/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 '}'}}
Index: cfe/trunk/lib/Parse/ParseExprCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp
+++ cfe/trunk/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://lis

[PATCH] D39438: [analyzer] Diagnose stack leaks via block captures

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added a comment.

ping @NoQ


Repository:
  rL LLVM

https://reviews.llvm.org/D39438



___
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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! Mostly just comments around the IO (which I'm still learning about 
myself).
Otherwise this LG.

I'm afraid you're going to run into some merge conflicts with 
https://reviews.llvm.org/rL317486 - I think/hope not too bad, though.




Comment at: clangd/ClangdServer.cpp:41
   // 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)

rwols wrote:
> sammccall wrote:
> > This needs to use the VFS I think. (I'm not familiar with the details of 
> > the VFS usage I'm afraid)
> Haven't tried implementing this yet, but from some simple experiments it 
> doesn't seem necessary.
Thanks for doing this. The reason it's needed is we want to run clangd on 
systems where the source tree is not a real filesystem. Looking on the real 
filesystem for .clang-format files would be incorrect.



Comment at: clangd/ClangdServer.cpp:297
 
-std::vector ClangdServer::formatRange(PathRef File,
-Range Rng) {
-  std::string Code = getDocument(File);
-
+llvm::Expected>>
+ClangdServer::formatRange(llvm::StringRef Code, PathRef File, Range Rng) {

As I understand, we tag the return values to ensure we get consistent reads, 
and describe the consistency of those reads, on systems that support that (e.g. 
snapshot filesystems).

I don't think we need to do that here, because:
 - formatting only depends on the formatted file, and .clang-format
 - the formatted file's contents are passed directly
 - we should aggressively cache .clang-format reads, I think (see below)



Comment at: clangd/ClangdServer.cpp:429
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  auto StyleOrError =
+  format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());

getStyle is going to stat several files, in the usual case.
This seems less than ideal, particularly when we're doing format-on-type. 
Filesystems are not always fast/cheap.

We should really cache this, and I think "when the file is opened" is a 
reasonable tradeoff between simplicity and producing sensible user-facing 
behavior.

Unfortunately I'm not sure there's a sensible place to do things at present: 
addDocument() is fast and doing blocking IO there might be bad. Doing it 
asynchronously also seems bad (what happens if style's not ready?)

Next best thing would be "first format request when the file is opened". If you 
feel like doing that here (only computing the style if it's not already known 
for this file), then feel free, but a **FIXME** is fine too.

(FWIW, Google has a slow FS that's important to us, but we disable 
.clang-format scanning on that FS in other ways because it only holds code in 
one style. Other users might care about this, though)



Comment at: clangd/ClangdServer.h:314
 private:
+  // FIXME: We don't need the Code argument, but that data just so happens to
+  // be present when invoking this method.

I don't understand this comment - I think we require and use Code.

If you mean we can just read Code from disk, that won't work: we want to format 
the dirty buffer in memory, not the last saved version.


https://reviews.llvm.org/D39430



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

[PATCH] D39676: [clangd] Add rename support.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Impl LG apart from error handling :-)




Comment at: clangd/ClangdServer.cpp:338
 
+std::vector
+ClangdServer::Rename(PathRef File, Position Pos, llvm::StringRef NewName) {

I think you can split out a private method:

Expected> 
ClangdServer::applyRefactoring(RefactoringActionRuleBase&);

This would make this function easier to read by separating out the 
rename-specific stuff from (some of) the boilerplate.
(It also seems likely to be reusable if we add direct support for other 
refactorings, but that's not the main reason I think)



Comment at: clangd/ClangdServer.cpp:345
+  public:
+void handleError(llvm::Error Err) override {}
+

don't you want to do something here?



Comment at: clangd/ClangdServer.cpp:347
+
+void handle(tooling::SymbolOccurrences) override {}
+

I don't think you need to override this, assuming you don't expect any of these



Comment at: clangd/ClangdServer.cpp:369
+Context, SourceRange(SourceLocationBeg), NewName.str());
+if (rename)
+   rename->invoke(ResultCollector, Context);

else?



Comment at: clangd/ClangdServer.cpp:377
+  // FIXME: Right now we only support renaming the main file, so we drop
+  // replacements not for the main file.
+  if (Rep.getFilePath() == File)

Add to the comment what you actually want to fix :-)
Things we may want:
 - rename in any included header
 - rename only in the "main" header
 - provide an error if there are decls we won't rename (e.g. if you rename 
std::vector)
 - rename globally in project (this may be slow or surprising, so I'd make that 
a more explicit action)
 - rename in open files (though I'm leery about side-effects of open files)



Comment at: clangd/ClangdUnit.cpp:1399
+
+SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
+const FileEntry *FE) {

nit: the rest of this file defines functions outside their namespace.

TBH I prefer the style you're using here, but we should be consistent within a 
file.



Comment at: test/clangd/rename.test:1
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.

For new tests after rL317486, please add -pretty and FileCheck 
-strict-whitespace

(See that patch for how the CHECKs should look)



Comment at: test/clangd/rename.test:14
+#
+
+Content-Length: 159

uber-nit: the # line previous to this one is for spacing, no need for another 
blank line (or any of them, as far as I'm concerned)



Comment at: test/clangd/rename.test:25
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+Content-Length: 33

No need to assert the response from shutdown (I removed most of these recently)


https://reviews.llvm.org/D39676



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

[PATCH] D39631: [X86] Fix the spelling of 3dnow and 3dnowa in isValidFeatureName

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D39631



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

[PATCH] D39505: [OpenMP] Show error if VLAs are not supported

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:944
+  /// \brief Whether target supports variable-length arrays.
+  bool isVLASupported() const { return VLASupported; }
+

ABataev wrote:
> Hahnfeld wrote:
> > rjmccall wrote:
> > > Hahnfeld wrote:
> > > > rjmccall wrote:
> > > > > Hahnfeld wrote:
> > > > > > rjmccall wrote:
> > > > > > > The way you've written this makes it sound like "does the target 
> > > > > > > support VLAs?", but the actual semantic checks treat it as "do 
> > > > > > > OpenMP devices on this target support VLAs?"  Maybe there should 
> > > > > > > be a more specific way to query things about OpenMP devices 
> > > > > > > instead of setting a global flag for the target?
> > > > > > Actually, the NVPTX and SPIR targets never support VLAs. So I felt 
> > > > > > like it would be more correct to make this a global property of the 
> > > > > > target.
> > > > > > 
> > > > > > The difference is that the other programming models (OpenCL and 
> > > > > > CUDA) error out immediatelyand regardless of the target because 
> > > > > > this limitation is reflected in the standards that disallow VLAs 
> > > > > > (see SemaType.cpp). For OpenMP we might have target devices that 
> > > > > > support VLA so we shouldn't error out for those.
> > > > > If you want to make it a global property of the target, that's fine, 
> > > > > but then I don't understand why your diagnostic only fires when 
> > > > > (S.isInOpenMPDeclareTargetContext() || 
> > > > > S.isInOpenMPTargetExecutionDirective()).
> > > > That is because of how OpenMP offloading works and how it is 
> > > > implemented in Clang. Consider the following snippet from the added 
> > > > test case:
> > > > ```lang=c
> > > > int vla[arg];
> > > > 
> > > > #pragma omp target map(vla[0:arg])
> > > > {
> > > >// more code here...
> > > > }
> > > > ```
> > > > 
> > > > Clang will take the following steps to compile this into a working 
> > > > binary for a GPU:
> > > > 1. Parse and (semantically) analyze the code as-is for the host and 
> > > > produce LLVM Bitcode.
> > > > 2. Parse and analyze again the code as-is and generate code for the 
> > > > offloading target, the GPU in this case.
> > > > 3. Take LLVM Bitcode from 1., generate host binary and embed target 
> > > > binary from 3.
> > > > 
> > > > `OpenMPIsDevice` will be true for 2., but the complete source code is 
> > > > analyzed. So to not throw errors for the host code, we have to make 
> > > > sure that we are actually generating code for the target device. This 
> > > > is either in a `target` directive or in a `declare target` region.
> > > > Note that this is quite similar to what CUDA does, only they have 
> > > > `CUDADiagIfDeviceCode` for this logic. If you want me to add something 
> > > > of that kind for OpenMP target devices, I'm fine with that. However for 
> > > > the given case, it's a bit different because this error should only be 
> > > > thrown for target devices that don't support VLAs...
> > > I see.  So the entire translation unit is re-parsed and re-Sema'ed from 
> > > scratch for the target?  Which means you need to avoid generating errors 
> > > about things in the outer translation unit that aren't part of the target 
> > > directive that you actually want to compile.  I would've expected there 
> > > to be some existing mechanism for that, to be honest, as opposed to 
> > > explicitly trying to suppress target-specific diagnostics one by one.
> > Yes, that is my understanding. For errors, we don't need to take anything 
> > special as the first `cc1` invocation will exit with a non-zero status so 
> > that the driver stops the compilation. For warnings, there seems to be no 
> > mechanism in place as I see them duplicated, even in code that is not 
> > generate for the target device (verified with an unused variable).
> > 
> > @ABataev @gtbercea Do I miss something here?
> I'm not aware of any.
John, target-specific checks require some special flags (like LangOpts.Cuda) 
that are not set when we re-compile the code for OpenMP devices. That's why 
errors are not emitted for the non-target code. But also because of that, we 
need some special OpenMP checks for target-specific code inside the target 
regions. For example, code in lib/Sema/SemaType.cpp, lines 2184, 2185 (see this 
file in this patch) checks for Cuda compilation and prohibits using of VLAs in 
Cuda mode. We also should prohibit using of VLAs in target code for NVPTX 
devices or other devices that do not support VLAs in OpenMP mode.


https://reviews.llvm.org/D39505



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: lib/Headers/avx512vlbwintrin.h:2801
+  return _mm_mask_cmp_epi8_mask (__U, _mm_and_si128 (__A, __B),
+ _mm_setzero_hi(), 4);
 }

Sorry I didn't catch this before, but can you use _MM_CMPINT_NE and 
_MM_CMPINT_EQ instead of '4' and '0' in all of these? Then no one has to guess 
what 4 and 0 means.


https://reviews.llvm.org/D38737



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

[PATCH] D38737: [X86] test/testn intrinsics lowering to IR. clang side

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: lib/Headers/avx512vlbwintrin.h:2801
+  return _mm_mask_cmp_epi8_mask (__U, _mm_and_si128 (__A, __B),
+ _mm_setzero_hi(), 4);
 }

craig.topper wrote:
> Sorry I didn't catch this before, but can you use _MM_CMPINT_NE and 
> _MM_CMPINT_EQ instead of '4' and '0' in all of these? Then no one has to 
> guess what 4 and 0 means.
Better yet, use _mm512_cmpneq_epi32_mask, __mm512_cmpeq_epi32_mask and friends


https://reviews.llvm.org/D38737



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

[PATCH] D39691: [analyzer] Model correct dispatch_once() 'done' value in BodyFarm

[George Karpenkov via Phabricator via cfe-commits]

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

Looks good to me.
I really don't think the test format chosen in `unix-fns.c` is a good idea, as 
it is very large, auto-generated, and tightly coupled to chosen formatting. I 
think combination of checking for warning with `clang_analyzer_eval` will be 
much better, but then maybe it's not a job for this patch.


https://reviews.llvm.org/D39691



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

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D33722#916540, @xazax.hun wrote:

> Also, bugprone might be a better module to put this?


I don't have strong opinions on misc vs bugprone (they're both effectively 
catch-alls for tidy checks, as best I can tell).


https://reviews.llvm.org/D33722



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

[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM, but wait a couple days before you commit to see if anyone else wants to 
comment.


https://reviews.llvm.org/D39641



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

r317504 - [X86] Add 3dnow and 3dnowa to the list of valid target features

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Mon Nov  6 12:33:13 2017
New Revision: 317504

URL: http://llvm.org/viewvc/llvm-project?rev=317504&view=rev
Log:
[X86] Add 3dnow and 3dnowa to the list of valid target features

These were missed in SVN r316783, which broke compiling mingw-w64 CRT.

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

Added:
cfe/trunk/test/Headers/mm3dnow.c
Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=317504&r1=317503&r2=317504&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Nov  6 12:33:13 2017
@@ -1121,6 +1121,8 @@ void X86TargetInfo::getTargetDefines(con
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::StringSwitch(Name)
+  .Case("3dnow", true)
+  .Case("3dnowa", true)
   .Case("aes", true)
   .Case("avx", true)
   .Case("avx2", true)

Added: cfe/trunk/test/Headers/mm3dnow.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/mm3dnow.c?rev=317504&view=auto
==
--- cfe/trunk/test/Headers/mm3dnow.c (added)
+++ cfe/trunk/test/Headers/mm3dnow.c Mon Nov  6 12:33:13 2017
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding %s -verify
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -x c++ %s -verify
+// expected-no-diagnostics
+
+#if defined(i386) || defined(__x86_64__)
+#include 
+
+int __attribute__((__target__(("3dnow" foo(int a) {
+  _m_femms();
+  return 4;
+}
+
+__m64 __attribute__((__target__(("3dnowa" bar(__m64 a) {
+  return _m_pf2iw(a);
+}
+#endif


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

[PATCH] D39631: [X86] Fix the spelling of 3dnow and 3dnowa in isValidFeatureName

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317504: [X86] Add 3dnow and 3dnowa to the list of valid 
target features (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D39631?vs=121623&id=121766#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39631

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/test/Headers/mm3dnow.c


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1121,6 +1121,8 @@
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::StringSwitch(Name)
+  .Case("3dnow", true)
+  .Case("3dnowa", true)
   .Case("aes", true)
   .Case("avx", true)
   .Case("avx2", true)
Index: cfe/trunk/test/Headers/mm3dnow.c
===
--- cfe/trunk/test/Headers/mm3dnow.c
+++ cfe/trunk/test/Headers/mm3dnow.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding %s -verify
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -x c++ %s -verify
+// expected-no-diagnostics
+
+#if defined(i386) || defined(__x86_64__)
+#include 
+
+int __attribute__((__target__(("3dnow" foo(int a) {
+  _m_femms();
+  return 4;
+}
+
+__m64 __attribute__((__target__(("3dnowa" bar(__m64 a) {
+  return _m_pf2iw(a);
+}
+#endif


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1121,6 +1121,8 @@
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::StringSwitch(Name)
+  .Case("3dnow", true)
+  .Case("3dnowa", true)
   .Case("aes", true)
   .Case("avx", true)
   .Case("avx2", true)
Index: cfe/trunk/test/Headers/mm3dnow.c
===
--- cfe/trunk/test/Headers/mm3dnow.c
+++ cfe/trunk/test/Headers/mm3dnow.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding %s -verify
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -x c++ %s -verify
+// expected-no-diagnostics
+
+#if defined(i386) || defined(__x86_64__)
+#include 
+
+int __attribute__((__target__(("3dnow" foo(int a) {
+  _m_femms();
+  return 4;
+}
+
+__m64 __attribute__((__target__(("3dnowa" bar(__m64 a) {
+  return _m_pf2iw(a);
+}
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=PREFIX

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.

This mimic's FileCheck's --check-prefix option.

The default PREFIX is "expected".  That is, "-verify" is equivalent to
"-verify=expected".

The goal is to permit exercising a single test suite source file with
different compiler options producing different sets of diagnostics.
While cpp can be combined with the existing -verify to accomplish the
same goal, source is often easier to read when it's not cluttered with
preprocessor directives.  For those cases, this tiny extension seems
worthwhile.


https://reviews.llvm.org/D39694

Files:
  include/clang/Basic/DiagnosticOptions.h
  include/clang/Driver/CC1Options.td
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/VerifyDiagnosticConsumer.cpp
  test/Frontend/verify-prefix.c

Index: test/Frontend/verify-prefix.c
===
--- /dev/null
+++ test/Frontend/verify-prefix.c
@@ -0,0 +1,38 @@
+// gconst-note@2 {{variable 'glb' declared const here}}
+GC int glb = 5;
+
+// Check various -verify prefixes.
+// RUN: %clang_cc1 -DGC=const -DLC=  -DSC= -verify=gconst %s
+// RUN: %clang_cc1 -Wcast-qual -DGC=  -DLC=const -DSC= -verify=lconst %s
+// RUN: %clang_cc1 -DGC=  -DLC=  -DSC= -verify=nconst %s
+
+// Check empty -verify prefix.  Make sure there's no additional output, which
+// might indicate diagnostic verification became enabled even though it was
+// requested incorrectly.
+// RUN: not %clang_cc1 -DGC=const -DLC=const -DSC=const -verify= %s 2> %t
+// RUN: FileCheck --check-prefix=EMPTY %s < %t
+// EMPTY-NOT: {{.}}
+// EMPTY: error: invalid value '' in '-verify='
+// EMPTY-NOT: {{.}}
+
+// Check omitting the -verify prefix.
+// RUN: %clang_cc1 -DGC= -DLC= -DSC=const -verify %s
+
+// Check omitting -verify: check that code actually has expected diagnostics.
+// RUN: not %clang_cc1 -Wcast-qual -DGC=const -DLC=const -DSC=const %s 2> %t
+// RUN: FileCheck --check-prefix=ALL %s < %t
+// ALL: cannot assign to variable 'glb' with const-qualified type 'const int'
+// ALL: variable 'glb' declared const here
+// ALL: cast from 'const int *' to 'int *' drops const qualifier
+// ALL: initializing 'int *' with an expression of type 'const int *' discards qualifiers
+
+void foo() {
+  LC int loc = 5;
+  SC static int sta = 5;
+  glb = 6; // gconst-error {{cannot assign to variable 'glb' with const-qualified type 'const int'}}
+  // lconst-warning@+1 {{cast from 'const int *' to 'int *' drops const qualifier}}
+  *(int*)(&loc) = 6;
+  int *p = &sta; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+}
+
+// nconst-no-diagnostics
Index: lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -315,7 +315,7 @@
   bool FoundDirective = false;
   for (ParseHelper PH(S); !PH.Done();) {
 // Search for token: expected
-if (!PH.Search("expected", true))
+if (!PH.Search(Diags.getDiagnosticOptions().VerifyPrefix, true))
   break;
 PH.Advance();
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1146,7 +1146,17 @@
   Opts.ShowSourceRanges = Args.hasArg(OPT_fdiagnostics_print_source_range_info);
   Opts.ShowParseableFixits = Args.hasArg(OPT_fdiagnostics_parseable_fixits);
   Opts.ShowPresumedLoc = !Args.hasArg(OPT_fno_diagnostics_use_presumed_location);
-  Opts.VerifyDiagnostics = Args.hasArg(OPT_verify);
+  Opts.VerifyDiagnostics = Args.hasArg(OPT_verify) || Args.hasArg(OPT_verify_EQ);
+  Opts.VerifyPrefix = Args.getLastArgValue(OPT_verify_EQ, "expected");
+  if (Opts.VerifyPrefix.empty()) {
+Success = false;
+Opts.VerifyDiagnostics = false;
+if (Diags) {
+  Arg *A = Args.getLastArg(OPT_verify_EQ);
+  Diags->Report(diag::err_drv_invalid_value)
+<< A->getAsString(Args) << A->getValue();
+}
+  }
   DiagnosticLevelMask DiagMask = DiagnosticLevelMask::None;
   Success &= parseDiagnosticLevelMask("-verify-ignore-unexpected=",
 Args.getAllArgValues(OPT_verify_ignore_unexpected_EQ),
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -398,8 +398,11 @@
   HelpText<"Set the maximum number of source lines to show in a caret diagnostic">;
 def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"">,
   HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
+def verify_EQ : Joined<["-"], "verify=">,
+  MetaVarName<"">,
+  HelpText<"Verify diagnostic output using comment directives that start with ">;
 def verify : Flag<["-"], "verify">,
-  HelpText<"Verify diagnostic output usi

[PATCH] D39620: [analyzer] [NFC] Remove unused typedef

[Devin Coughlin via Phabricator via cfe-commits]

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

Looks good to me.


https://reviews.llvm.org/D39620



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

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak added a comment.

@djasper: ping :)


https://reviews.llvm.org/D39478



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

[PATCH] D39611: [CodeGen] make cbrt and fma constant (never set errno); document complex calls as always constant

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Currently, the clang driver supports three platforms which have errno-setting 
libc implementations: GNU/Linux (with glibc), Solaris, and Windows (with Visual 
Studio CRT).  (BSD-based systems, including OS X, never set errno in libm, so 
they aren't relevant here.)  As long as our markings are consistent with those 
targets, we should be fine.

Both MSVC and GNU libraries never set errno for cbrt() and fma(); we want to 
keep those assumptions in clang.

We can ignore Visual Studio for the discussion of complex.h, I guess, because 
MSVC doesn't support _Complex.  (It has a header called complex.h, but the 
types aren't right, so clang won't recognize any of the functions.  For 
reference, though, its cabs() and catanh() set errno).

I have no idea how Solaris and other Unix targets handle these functions; it's 
probably a bad idea to add special cases for them without testing.


https://reviews.llvm.org/D39611



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

r317511 - Vary Windows toolchain selection by -fuse-ld

[Dave Lee via cfe-commits]

Author: kastiglione
Date: Mon Nov  6 13:18:05 2017
New Revision: 317511

URL: http://llvm.org/viewvc/llvm-project?rev=317511&view=rev
Log:
Vary Windows toolchain selection by -fuse-ld

Summary:
This change allows binutils to be used for linking with MSVC. Currently, when
using an MSVC target and `-fuse-ld=bfd`, the driver produces an invalid linker
invocation.

Reviewers: rnk, compnerd

Reviewed By: compnerd

Subscribers: smeenai, cfe-commits

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

Added:
cfe/trunk/test/Driver/Inputs/Windows/usr/
cfe/trunk/test/Driver/Inputs/Windows/usr/bin/
cfe/trunk/test/Driver/Inputs/Windows/usr/bin/ld.bfd   (with props)
Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/fuse-ld.c

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=317511&r1=317510&r2=317511&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Mon Nov  6 13:18:05 2017
@@ -3880,7 +3880,13 @@ const ToolChain &Driver::getToolChain(co
 break;
   case llvm::Triple::MSVC:
   case llvm::Triple::UnknownEnvironment:
-TC = llvm::make_unique(*this, Target, Args);
+if (Args.getLastArgValue(options::OPT_fuse_ld_EQ)
+.startswith_lower("bfd"))
+  TC = llvm::make_unique(
+  *this, Target, Args);
+else
+  TC =
+  llvm::make_unique(*this, Target, 
Args);
 break;
   }
   break;

Added: cfe/trunk/test/Driver/Inputs/Windows/usr/bin/ld.bfd
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/Windows/usr/bin/ld.bfd?rev=317511&view=auto
==
(empty)

Propchange: cfe/trunk/test/Driver/Inputs/Windows/usr/bin/ld.bfd
--
svn:executable = *

Modified: cfe/trunk/test/Driver/fuse-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuse-ld.c?rev=317511&r1=317510&r2=317511&view=diff
==
--- cfe/trunk/test/Driver/fuse-ld.c (original)
+++ cfe/trunk/test/Driver/fuse-ld.c Mon Nov  6 13:18:05 2017
@@ -67,3 +67,29 @@
 // RUN: -gcc-toolchain %S/Inputs/basic_android_tree 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ANDROID-ARM-GOLD-TC
 // CHECK-ANDROID-ARM-GOLD-TC: 
Inputs/basic_android_tree/lib/gcc/arm-linux-androideabi/4.4.3/../../../../arm-linux-androideabi/bin{{/|\\+}}ld.gold
+
+
+// RUN: %clang %s -### -fuse-ld=link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LINK
+// CHECK-WINDOWS-MSVC-LINK: "{{.*}}link.exe"
+// CHECK-WINDOWS-MSVC-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD
+// CHECK-WINDOWS-MSVC-LLD: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld-link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD-LINK
+// CHECK-WINDOWS-MSVC-LLD-LINK: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=bfd \
+// RUN: -target i686-unknown-windows-msvc \
+// RUN: -B %S/Inputs/Windows/usr/bin 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
+// CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
+// CHECK-WINDOWS-MSVC-BFD-SAME: "-o"


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

[PATCH] D39509: Vary Windows toolchain selection by -fuse-ld

[Dave Lee via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317511: Vary Windows toolchain selection by -fuse-ld 
(authored by kastiglione).

Repository:
  rL LLVM

https://reviews.llvm.org/D39509

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/Inputs/Windows/usr/bin/ld.bfd
  cfe/trunk/test/Driver/fuse-ld.c


Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -67,3 +67,29 @@
 // RUN: -gcc-toolchain %S/Inputs/basic_android_tree 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ANDROID-ARM-GOLD-TC
 // CHECK-ANDROID-ARM-GOLD-TC: 
Inputs/basic_android_tree/lib/gcc/arm-linux-androideabi/4.4.3/../../../../arm-linux-androideabi/bin{{/|\\+}}ld.gold
+
+
+// RUN: %clang %s -### -fuse-ld=link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LINK
+// CHECK-WINDOWS-MSVC-LINK: "{{.*}}link.exe"
+// CHECK-WINDOWS-MSVC-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD
+// CHECK-WINDOWS-MSVC-LLD: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld-link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD-LINK
+// CHECK-WINDOWS-MSVC-LLD-LINK: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=bfd \
+// RUN: -target i686-unknown-windows-msvc \
+// RUN: -B %S/Inputs/Windows/usr/bin 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
+// CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
+// CHECK-WINDOWS-MSVC-BFD-SAME: "-o"
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3880,7 +3880,13 @@
 break;
   case llvm::Triple::MSVC:
   case llvm::Triple::UnknownEnvironment:
-TC = llvm::make_unique(*this, Target, Args);
+if (Args.getLastArgValue(options::OPT_fuse_ld_EQ)
+.startswith_lower("bfd"))
+  TC = llvm::make_unique(
+  *this, Target, Args);
+else
+  TC =
+  llvm::make_unique(*this, Target, 
Args);
 break;
   }
   break;


Index: cfe/trunk/test/Driver/fuse-ld.c
===
--- cfe/trunk/test/Driver/fuse-ld.c
+++ cfe/trunk/test/Driver/fuse-ld.c
@@ -67,3 +67,29 @@
 // RUN: -gcc-toolchain %S/Inputs/basic_android_tree 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ANDROID-ARM-GOLD-TC
 // CHECK-ANDROID-ARM-GOLD-TC: Inputs/basic_android_tree/lib/gcc/arm-linux-androideabi/4.4.3/../../../../arm-linux-androideabi/bin{{/|\\+}}ld.gold
+
+
+// RUN: %clang %s -### -fuse-ld=link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LINK
+// CHECK-WINDOWS-MSVC-LINK: "{{.*}}link.exe"
+// CHECK-WINDOWS-MSVC-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD
+// CHECK-WINDOWS-MSVC-LLD: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=lld-link \
+// RUN: -target i686-unknown-windows-msvc 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-LLD-LINK
+// CHECK-WINDOWS-MSVC-LLD-LINK: "{{.*}}lld-link"
+// CHECK-WINDOWS-MSVC-LLD-LINK-SAME: "-out:{{.*}}"
+
+// RUN: %clang %s -### -fuse-ld=bfd \
+// RUN: -target i686-unknown-windows-msvc \
+// RUN: -B %S/Inputs/Windows/usr/bin 2>&1 \
+// RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
+// CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
+// CHECK-WINDOWS-MSVC-BFD-SAME: "-o"
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3880,7 +3880,13 @@
 break;
   case llvm::Triple::MSVC:
   case llvm::Triple::UnknownEnvironment:
-TC = llvm::make_unique(*this, Target, Args);
+if (Args.getLastArgValue(options::OPT_fuse_ld_EQ)
+.startswith_lower("bfd"))
+  TC = llvm::make_unique(
+  *this, Target, Args);
+else
+  TC =
+  llvm::make_unique(*this, Target, Args);
 break;
   }
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:26
+/// results in an ElementRegion.
+static void conjureOffsetSymbolOnLocation(
+SVal &Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder,

I think it would be more clear at the call site what the inputs and output are 
if this function returned the conjured SVal (or 'Symbol' if it wouldn't be 
conjured).

It would also be good to describe the role of the parameters in the doxygen 
comment.


https://reviews.llvm.org/D39584



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D38680#903203, @joerg wrote:

> I've looked at this in some detail now. I'm not exactly sure yet why it is 
> broken. The patch seems quite wrong to me. DW_CFA_GNU_args_size should be 
> applied only when unwinding a call instruction and that regard, the commit 
> message of the original change is quite correct. What I am still trying to 
> understand is how the precise unwind frame disagrees with the unwinder.


@joerg Any update on this matter, how should we move forward with unbreaking 
the unwinding on i386?


https://reviews.llvm.org/D38680



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 121779.
steven_wu added a comment.

This seems to be the cleanest solution I can find for CUDA without
touching the job configuration for CUDA. My proposed fix is to bail
out of the jobs that are parts of CUDA offload if some command failed
before that. Now if you hit failures when compiling multiple GPU
architectures, you will error out only once. This is pretty much the exact
same behavior of current driver.

This is not the ideal solution but there is simply no information in the job
setup in current CUDA driver to detect if the input is same across different
jobs. For all the CUDA inputs, it is duplicated into different InputActions
and insert into different OffloadActions. You also cannot detect them by
looking at their name because the following should compiled the same file
twice even there are errors in it:

  cc a.c a.c
  `

Also, the reason I don't know how to craft a testcase is not because I have
trouble with CUDA driver, but how to write a test to check when did the driver
bailed out. Let me know if you have any suggestions.


https://reviews.llvm.org/D39502

Files:
  lib/Driver/Compilation.cpp
  test/Driver/output-file-cleanup.c
  test/Driver/unix-conformance.c

Index: test/Driver/unix-conformance.c
===
--- /dev/null
+++ test/Driver/unix-conformance.c
@@ -0,0 +1,24 @@
+// Check UNIX conformance for cc/c89/c99
+// When c99 encounters a compilation error that causes an object file not to be
+// created, it shall write a diagnostic to standard error and continue to
+// compile other source code operands, but it shall not perform the link phase
+// and it shall return a non-zero exit status.
+
+// When given multiple .c files to compile, clang compiles them in order until
+// it hits an error, at which point it stops.
+//
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir
+// RUN: cd %t-dir
+//
+// RUN: touch %t-dir/1.c
+// RUN: echo "invalid C code" > %t-dir/2.c
+// RUN: touch %t-dir/3.c
+// RUN: echo "invalid C code" > %t-dir/4.c
+// RUN: touch %t-dir/5.c
+// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
+// RUN: test -f %t-dir/1.s
+// RUN: test ! -f %t-dir/2.s
+// RUN: test -f %t-dir/3.s
+// RUN: test ! -f %t-dir/4.s
+// RUN: test -f %t-dir/5.s
Index: test/Driver/output-file-cleanup.c
===
--- test/Driver/output-file-cleanup.c
+++ test/Driver/output-file-cleanup.c
@@ -41,18 +41,3 @@
 // RUN: not %clang -S %t-dir/1.c %t-dir/2.c
 // RUN: test -f %t-dir/1.s
 // RUN: test ! -f %t-dir/2.s
-
-// When given multiple .c files to compile, clang compiles them in order until
-// it hits an error, at which point it stops.
-//
-// RUN: touch %t-dir/1.c
-// RUN: echo "invalid C code" > %t-dir/2.c
-// RUN: touch %t-dir/3.c
-// RUN: echo "invalid C code" > %t-dir/4.c
-// RUN: touch %t-dir/5.c
-// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
-// RUN: test -f %t-dir/1.s
-// RUN: test ! -f %t-dir/2.s
-// RUN: test ! -f %t-dir/3.s
-// RUN: test ! -f %t-dir/4.s
-// RUN: test ! -f %t-dir/5.s
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -182,16 +182,51 @@
   return ExecutionFailed ? 1 : Res;
 }
 
-void Compilation::ExecuteJobs(
-const JobList &Jobs,
-SmallVectorImpl> &FailingCommands) const {
+using FailingCommandList = SmallVectorImpl>;
+
+static bool ActionFailed(const Action *A,
+ const FailingCommandList &FailingCommands) {
+
+  if (FailingCommands.empty())
+return false;
+
+  // CUDA can have the same input source code compiled multiple times so do not
+  // compiled again if there are already failures. It is OK to abort the CUDA
+  // pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda))
+return true;
+
+  for (const auto &CI : FailingCommands)
+if (A == &(CI.second->getSource()))
+  return true;
+
+  for (const Action *AI : A->inputs())
+if (ActionFailed(AI, FailingCommands))
+  return true;
+
+  return false;
+}
+
+static bool InputsOk(const Command &C,
+ const FailingCommandList &FailingCommands) {
+  return !ActionFailed(&C.getSource(), FailingCommands);
+}
+
+void Compilation::ExecuteJobs(const JobList &Jobs,
+  FailingCommandList &FailingCommands) const {
+  // According to UNIX standard, driver need to continue compiling all the
+  // inputs on the command line even one of them failed.
+  // In all but CLMode, execute all the jobs unless the necessary inputs for the
+  // job is missing due to previous failures.
   for (const auto &Job : Jobs) {
+if (!InputsOk(Job, FailingCommands))
+  continue;
 const Command *FailingCommand = nullptr;
 if (int Res = ExecuteCommand(Job, FailingCommand)) {
   FailingCommands.push_back(std::make_pair(Res,

[PATCH] D39691: [analyzer] Model correct dispatch_once() 'done' value in BodyFarm

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

Yeah, I'm not a fan of this style of testing for path diagnostics, either.


https://reviews.llvm.org/D39691



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

[PATCH] D39676: [clangd] Add rename support.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Side-comment: will want to let the user know about the fact that global symbols 
aren't really renamed yet until global rename works?


https://reviews.llvm.org/D39676



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In https://reviews.llvm.org/D39502#917097, @steven_wu wrote:

> Also, the reason I don't know how to craft a testcase is not because I have
>  trouble with CUDA driver, but how to write a test to check when did the 
> driver
>  bailed out. Let me know if you have any suggestions.


Here's one way to do it: Run 'cc -v', positively match cc1 invocations that 
worked and CHECK-NOT for cc1 invocations that would happen after bail-out 
point. 
Use preprocessor to induce the failure during particular compilation phase:

  # if __CUDA_ARCH__ == 350
  #error Error during compilation for sm_35
  #endif
  
  #ifndef __CUDA_ARCH__
  #error Error during host compilation
  #endif


https://reviews.llvm.org/D39502



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

[PATCH] D39332: [clang-refactor] Introduce a new rename rule for qualified symbols

[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));

sammccall wrote:
> hokein wrote:
> > arphaman wrote:
> > > ioeric wrote:
> > > > hokein wrote:
> > > > > arphaman wrote:
> > > > > > 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());
> > > > > > ```
> > > > > Done. Introduced a common interface `USRRenameRule`.
> > > > `USRRenameRule` doesn't seem to be a very useful abstraction. I think 
> > > > what Alex proposed is a base class that implements 
> > > > `createSourceReplacements` which could be shared by both 
> > > > `RenameOccurrences` and `QualifiedRenameRule`. Intuitively, 
> > > > un-qualified rename is a special case of qualified rename (maybe?), 
> > > > where namespace is not changed.
> > > Yep, I meant that indeed.
> > Ah, sorry for the misunderstanding. 
> > 
> > Unfortunately, `RenameOccurrences` and `QualifiedRenameRule` could not 
> > share the same `createSourceReplacements` implementation, 
> > 
> > * Their supported use cases are different. `QualifiedRenameRule` only 
> > supports renaming class, function, enums, alias and class members, which 
> > doesn't cover all the cases of `RenameOccurrences` (like renaming local 
> > variable in function body).
> > 
> > * we have separated implementations in the code 
> > base(`createRenameAtomicChanges` for qualified rename, 
> > `createRenameReplacements` for un-qualified rename). The implementation of 
> > qualified rename does more things, including calculating the shortest 
> > prefix qualifiers, getting correct range of replacement.  
> > 
> > 
> That makes sense (I think), but then we should remove  `USRRenameRule` again 
> if we're not actually reusing anything.
> 
> (And ideally we can hide any such future reuse as functions in the cc file, 
> rather than a class hierarchy exposed in the header)
ok, that's fine for now then.



Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:104
+  if (!ND)
+return llvm::make_error("Could not find symbol " +
+   OldQualifiedName,

You can use a refactoring diagnostic here (see `RenameOccurrences::initiate`).


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] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D38680#903203, @joerg wrote:

> I've looked at this in some detail now. I'm not exactly sure yet why it is 
> broken. The patch seems quite wrong to me. DW_CFA_GNU_args_size should be 
> applied only when unwinding a call instruction and that regard, the commit 
> message of the original change is quite correct. What I am still trying to 
> understand is how the precise unwind frame disagrees with the unwinder.


If you look at Clang's output here https://godbolt.org/g/jFcSxz, you can see 
that we emit precise CFA adjustments for each push. We don't need to adjust the 
CFA by gnu arg size in UnwindCursor::step, which unwinds through frames. We 
only apply it when setting up the register context before transitioning to the 
landingpad. That's why unw_set_reg UNW_REG_IP is at least approximately the 
right place to do this SP adjustment, IMO.

Basically, a general purpose unwinder that collects return addresses can ignore 
gnu args size because we already have CFA adjustments, but an unwinder that 
implements landingpad transitions must adjust SP by gnu arg size as part of 
that context switch.

At least, that's how I understand this change. Maybe older (VAX?) compilers 
didn't implement gnu arg size this way, but at least with this patch, we 
handshake with ourselves correctly.


https://reviews.llvm.org/D38680



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

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added subscribers: cfe-commits, efriedma.
efriedma added a comment.

Adding cfe-commits.  (In the future, please make sure to CC that list instead 
of llvm-commits for clang changes.)

> I'm missing some background lingo here. What is the meaning of "a 
> declaration" and "a definition" here?

In most cases, we only emit the vtable for a class in one object file.  
Informally, we can call that the "definition" of the vtable, as an analogy to 
variables and functions.  See also 
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable .


Repository:
  rL LLVM

https://reviews.llvm.org/D39627



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 121795.
steven_wu added a comment.

Add testcase for CUDA driver


https://reviews.llvm.org/D39502

Files:
  lib/Driver/Compilation.cpp
  test/Driver/cuda-bail-out.cu
  test/Driver/output-file-cleanup.c
  test/Driver/unix-conformance.c

Index: test/Driver/unix-conformance.c
===
--- /dev/null
+++ test/Driver/unix-conformance.c
@@ -0,0 +1,24 @@
+// Check UNIX conformance for cc/c89/c99
+// When c99 encounters a compilation error that causes an object file not to be
+// created, it shall write a diagnostic to standard error and continue to
+// compile other source code operands, but it shall not perform the link phase
+// and it shall return a non-zero exit status.
+
+// When given multiple .c files to compile, clang compiles them in order until
+// it hits an error, at which point it stops.
+//
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir
+// RUN: cd %t-dir
+//
+// RUN: touch %t-dir/1.c
+// RUN: echo "invalid C code" > %t-dir/2.c
+// RUN: touch %t-dir/3.c
+// RUN: echo "invalid C code" > %t-dir/4.c
+// RUN: touch %t-dir/5.c
+// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
+// RUN: test -f %t-dir/1.s
+// RUN: test ! -f %t-dir/2.s
+// RUN: test -f %t-dir/3.s
+// RUN: test ! -f %t-dir/4.s
+// RUN: test -f %t-dir/5.s
Index: test/Driver/output-file-cleanup.c
===
--- test/Driver/output-file-cleanup.c
+++ test/Driver/output-file-cleanup.c
@@ -41,18 +41,3 @@
 // RUN: not %clang -S %t-dir/1.c %t-dir/2.c
 // RUN: test -f %t-dir/1.s
 // RUN: test ! -f %t-dir/2.s
-
-// When given multiple .c files to compile, clang compiles them in order until
-// it hits an error, at which point it stops.
-//
-// RUN: touch %t-dir/1.c
-// RUN: echo "invalid C code" > %t-dir/2.c
-// RUN: touch %t-dir/3.c
-// RUN: echo "invalid C code" > %t-dir/4.c
-// RUN: touch %t-dir/5.c
-// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
-// RUN: test -f %t-dir/1.s
-// RUN: test ! -f %t-dir/2.s
-// RUN: test ! -f %t-dir/3.s
-// RUN: test ! -f %t-dir/4.s
-// RUN: test ! -f %t-dir/5.s
Index: test/Driver/cuda-bail-out.cu
===
--- /dev/null
+++ test/Driver/cuda-bail-out.cu
@@ -0,0 +1,31 @@
+// Test cland driver bailed out after one error during CUDA compilation
+
+// REQUIRES: clang-driver
+// REQUIRES: powerpc-registered-target
+// REQUIRES: nvptx-registered-target
+
+# if __CUDA_ARCH__ == 350
+#error Error during compilation for sm_35
+#endif
+
+# if __CUDA_ARCH__ == 600
+#error Error during compilation for sm_60
+#endif
+
+#ifndef __CUDA_ARCH__
+#error Error during host compilation
+#endif
+
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -nocudalib -nocudainc \
+// RUN:   --cuda-gpu-arch=sm_35 %s 2>&1 | FileCheck %s
+
+// CHECK: Error during compilation for sm_35
+// CHECK-NOT: Error during host compilation
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -nocudalib -nocudainc \
+// RUN:   --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_60 %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-TWO-ARCHES %s
+// CHECK-TWO-ARCHES: Error during compilation for sm_35
+// CHECK-TWO-ARCHES-NOT: Error during compilation for sm_60
+// CHECK-TWO-ARCHES-NOT Error during host compilation
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -182,16 +182,51 @@
   return ExecutionFailed ? 1 : Res;
 }
 
-void Compilation::ExecuteJobs(
-const JobList &Jobs,
-SmallVectorImpl> &FailingCommands) const {
+using FailingCommandList = SmallVectorImpl>;
+
+static bool ActionFailed(const Action *A,
+ const FailingCommandList &FailingCommands) {
+
+  if (FailingCommands.empty())
+return false;
+
+  // CUDA can have the same input source code compiled multiple times so do not
+  // compiled again if there are already failures. It is OK to abort the CUDA
+  // pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda))
+return true;
+
+  for (const auto &CI : FailingCommands)
+if (A == &(CI.second->getSource()))
+  return true;
+
+  for (const Action *AI : A->inputs())
+if (ActionFailed(AI, FailingCommands))
+  return true;
+
+  return false;
+}
+
+static bool InputsOk(const Command &C,
+ const FailingCommandList &FailingCommands) {
+  return !ActionFailed(&C.getSource(), FailingCommands);
+}
+
+void Compilation::ExecuteJobs(const JobList &Jobs,
+  FailingCommandList &FailingCommands) const {
+  // According to UNIX standard, driver need to continue compiling all the
+  // inputs on the command line even one of them failed.
+  // In all but CLMode, execute all the jobs unless the necessary inputs for the
+  // job is missing due to previous failures.
   for (const auto &J

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In https://reviews.llvm.org/D39502#917132, @tra wrote:

> In https://reviews.llvm.org/D39502#917097, @steven_wu wrote:
>
> > Also, the reason I don't know how to craft a testcase is not because I have
> >  trouble with CUDA driver, but how to write a test to check when did the 
> > driver
> >  bailed out. Let me know if you have any suggestions.
>
>
> Here's one way to do it: Run 'cc -v', positively match cc1 invocations that 
> worked and CHECK-NOT for cc1 invocations that would happen after bail-out 
> point. 
>  Use preprocessor to induce the failure during particular compilation phase:
>
>   # if __CUDA_ARCH__ == 350
>   #error Error during compilation for sm_35
>   #endif
>  
>   #ifndef __CUDA_ARCH__
>   #error Error during host compilation
>   #endif
>


Thanks for the advice. Testcase added.


https://reviews.llvm.org/D39502



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: test/Driver/cuda-bail-out.cu:31
+// CHECK-TWO-ARCHES-NOT: Error during compilation for sm_60
+// CHECK-TWO-ARCHES-NOT Error during host compilation

Missing ':' after CHECK-TWO-ARCHES-NOT.

You may want to include a compilation for a GPU arch that does *not* fail. 
Otherwise this test would be happy if we'd always fail during first device-side 
compilation. I believe we compile GPU arches in sorted order, so using 
--cuda-gpu-arch=sm_30 should work for this purpose.

E.g. something like this:
```
#ifdef __CUDA_ARCH__ == 300
#warning  OK. No error during compilation for sm_30.
#endif
```


https://reviews.llvm.org/D39502



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

[PATCH] D39702: [test-suite] [CUDA] Delete nexttoward and nextafter CUDA tests, because these functions have no implementations.

[Justin Lebar via Phabricator via cfe-commits]

jlebar created this revision.
Herald added a subscriber: sanjoy.

These functions never actually worked -- if you called them, you'd get a
ptxas error when llvm tried to make a libcall to e.g. nexttowardf.

This test only compiled because the calls were DCE'ed.  Sadly my
laziness in not checking the return value of these functions resulted in
this long-standing bug.

On the upside, we check the return values of all other math functions,
so this bug is limited to nexttoward and nextafter.


https://reviews.llvm.org/D39702

Files:
  External/CUDA/cmath.cu
  External/CUDA/math_h.cu

Index: External/CUDA/math_h.cu
===
--- External/CUDA/math_h.cu
+++ External/CUDA/math_h.cu
@@ -86,8 +86,6 @@
 __device__ Ambiguous lrint(Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous lround(Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous nearbyint(Ambiguous){ return Ambiguous(); }
-__device__ Ambiguous nextafter(Ambiguous, Ambiguous){ return Ambiguous(); }
-__device__ Ambiguous nexttoward(Ambiguous, Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous remainder(Ambiguous, Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous remquo(Ambiguous, Ambiguous, int*){ return Ambiguous(); }
 __device__ Ambiguous rint(Ambiguous){ return Ambiguous(); }
@@ -1347,57 +1345,6 @@
 assert(nearbyint(1.f) == 1);
 }
 
-__device__ void test_nextafter()
-{
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-//assert(nextafter(0,1) == hexfloat(0x1, 0, -1074));
-
-// Invoke all our overloads, even if we can't be bothered to check the
-// results.
-nextafter(0, 1);
-nextafter(0, 1.);
-nextafter(0, 1.f);
-
-nextafter(0., 1);
-nextafter(0., 1.);
-nextafter(0., 1.f);
-
-nextafter(0.f, 1);
-nextafter(0.f, 1.);
-nextafter(0.f, 1.f);
-}
-
-__device__ void test_nexttoward()
-{
-static_assert((std::is_same::value), "");
-//assert(nexttoward(0, 1) == hexfloat(0x1, 0, -1074));
-
-// Invoke all our overloads, even if we can't be bothered to check the
-// results.
-nexttoward(0, 1);
-nexttoward(0, 1.);
-nexttoward(0, 1.f);
-
-nexttoward(0., 1);
-nexttoward(0., 1.);
-nexttoward(0., 1.f);
-
-nexttoward(0.f, 1);
-nexttoward(0.f, 1.);
-nexttoward(0.f, 1.f);
-}
-
 __device__ void test_remainder()
 {
 static_assert((std::is_same::value), "");
@@ -1647,8 +1594,6 @@
 test_lround();
 test_nan();
 test_nearbyint();
-test_nextafter();
-test_nexttoward();
 test_remainder();
 test_remquo();
 test_rint();
Index: External/CUDA/cmath.cu
===
--- External/CUDA/cmath.cu
+++ External/CUDA/cmath.cu
@@ -88,8 +88,6 @@
 __device__ Ambiguous lrint(Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous lround(Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous nearbyint(Ambiguous){ return Ambiguous(); }
-__device__ Ambiguous nextafter(Ambiguous, Ambiguous){ return Ambiguous(); }
-__device__ Ambiguous nexttoward(Ambiguous, Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous remainder(Ambiguous, Ambiguous){ return Ambiguous(); }
 __device__ Ambiguous remquo(Ambiguous, Ambiguous, int*){ return Ambiguous(); }
 __device__ Ambiguous rint(Ambiguous){ return Ambiguous(); }
@@ -1372,55 +1370,6 @@
 assert(std::nearbyint(1.f) == 1);
 }
 
-__device__ void test_nextafter()
-{
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-static_assert((std::is_same::value), "");
-
-// Invoke all our overloads, even if we can't be bothered to check the
-// results.
-std::nextafter(0, 1);
-std::nextafter(0, 1.);
-std::nextafter(0, 1.f);
-
-std::nextafter(0., 1);
-std::nextafter(0., 1.);
-std::nextafter(0., 1.f);
-
-std::nextafter(0.f, 1);
-std::nextafter(0.f, 1.);
-std::nextafter(0.f, 1.f);
-}
-
-__device__ void test_nexttoward()
-{
-static_assert((std::is_same::value), "");
-
-// Invoke all our overloads

[PATCH] D39703: [CUDA] Remove implementations of nexttoward and nextafter.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Libdevice does provide implementation for __nv_nextafterf() 
 and 
__nv_nextafter() 
 and it 
has corresponding wrappers in math_functions.h[pp].

Perhaps we should keep nextafter around.


https://reviews.llvm.org/D39703



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

[PATCH] D39706: [refactor][extract] Initial implementation of variable captures

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
Herald added a subscriber: mgorny.

This patch adds an initial rudimentary support for capturing variables that 
should be passed to the extracted function.
The variables are passed to the extracted function if they're used in the 
extracted code. If they're defined they're ignored for now, but we'll be able 
to handle variables that defined in extracted code but that are used after 
extracted code later. The variables are sorted by name when passing them to the 
extracted function. The exact passing semantics (reference/pointer/const 
reference/const pointer) are not yet computed.


Repository:
  rL LLVM

https://reviews.llvm.org/D39706

Files:
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/Extract/CaptureVariables.cpp
  lib/Tooling/Refactoring/Extract/CaptureVariables.h
  lib/Tooling/Refactoring/Extract/Extract.cpp
  test/Refactor/Extract/CaptureSimpleVariables.cpp
  test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
  test/Refactor/Extract/ExtractionSemicolonPolicy.m

Index: test/Refactor/Extract/ExtractionSemicolonPolicy.m
===
--- test/Refactor/Extract/ExtractionSemicolonPolicy.m
+++ test/Refactor/Extract/ExtractionSemicolonPolicy.m
@@ -10,7 +10,7 @@
   }
 }
 // CHECK: 1 'astmt' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(NSArray *array) {
 // CHECK-NEXT: for (id i in array) {
 // CHECK-NEXT: int x = 0;
 // CHECK-NEXT: }{{$}}
@@ -23,7 +23,7 @@
   }
 }
 // CHECK: 1 'bstmt' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(id lock) {
 // CHECK-NEXT: @synchronized(lock) {
 // CHECK-NEXT: int x = 0;
 // CHECK-NEXT: }{{$}}
Index: test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
===
--- test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
+++ test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
@@ -6,41 +6,41 @@
   /*range adeclstmt=->+0:59*/int area = r.width * r.height;
 }
 // CHECK: 1 'adeclstmt' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(const Rectangle &r) {
 // CHECK-NEXT: int area = r.width * r.height;{{$}}
 // CHECK-NEXT: }{{[[:space:]].*}}
 // CHECK-NEXT: void extractStatement(const Rectangle &r) {
-// CHECK-NEXT:   /*range adeclstmt=->+0:59*/extracted();{{$}}
+// CHECK-NEXT:   /*range adeclstmt=->+0:59*/extracted(r);{{$}}
 // CHECK-NEXT: }
 
 void extractStatementNoSemiIf(const Rectangle &r) {
   /*range bextractif=->+2:4*/if (r.width) {
 int x = r.height;
   }
 }
 // CHECK: 1 'bextractif' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(const Rectangle &r) {
 // CHECK-NEXT: if (r.width) {
 // CHECK-NEXT: int x = r.height;
 // CHECK-NEXT: }{{$}}
 // CHECK-NEXT: }{{[[:space:]].*}}
 // CHECK-NEXT: void extractStatementNoSemiIf(const Rectangle &r) {
-// CHECK-NEXT:   /*range bextractif=->+2:4*/extracted();{{$}}
+// CHECK-NEXT:   /*range bextractif=->+2:4*/extracted(r);{{$}}
 // CHECK-NEXT: }
 
 void extractStatementDontExtraneousSemi(const Rectangle &r) {
   /*range cextractif=->+2:4*/if (r.width) {
 int x = r.height;
   } ;
 } //^ This semicolon shouldn't be extracted.
 // CHECK: 1 'cextractif' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(const Rectangle &r) {
 // CHECK-NEXT: if (r.width) {
 // CHECK-NEXT: int x = r.height;
 // CHECK-NEXT: }{{$}}
 // CHECK-NEXT: }{{[[:space:]].*}}
 // CHECK-NEXT: void extractStatementDontExtraneousSemi(const Rectangle &r) {
-// CHECK-NEXT: extracted(); ;{{$}}
+// CHECK-NEXT: extracted(r); ;{{$}}
 // CHECK-NEXT: }
 
 void extractStatementNotSemiSwitch() {
@@ -102,12 +102,12 @@
   }
 }
 // CHECK: 1 'gextract' results:
-// CHECK:  static void extracted() {
+// CHECK:  static void extracted(XS xs) {
 // CHECK-NEXT: for (int i : xs) {
 // CHECK-NEXT: }{{$}}
 // CHECK-NEXT: }{{[[:space:]].*}}
 // CHECK-NEXT: void extractStatementNotSemiRangedFor(XS xs) {
-// CHECK-NEXT: extracted();{{$}}
+// CHECK-NEXT: extracted(xs);{{$}}
 // CHECK-NEXT: }
 
 void extractStatementNotSemiRangedTryCatch() {
Index: test/Refactor/Extract/CaptureSimpleVariables.cpp
===
--- /dev/null
+++ test/Refactor/Extract/CaptureSimpleVariables.cpp
@@ -0,0 +1,40 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
+
+void captureStaticVars() {
+  static int x;
+  /*range astaticvar=->+0:39*/int y = x;
+  x += 1;
+}
+// CHECK: 1 'astaticvar' results:
+// CHECK:  static void extracted(int x) {
+// CHECK-NEXT: int y = x;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void captureStaticVars() {
+// CHECK-NEXT: static int x;
+// CHECK-NEXT: extracted(x);{{$}}
+
+typedef struct {
+  int width, height;
+} Rectangle;
+
+void basicTypes(int i, float f, char c, const int *ip, float *fp, const Re

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 121807.
steven_wu added a comment.

Improve testcase according to review feedback.

In order to let compilation to be successful for one input, I need to use
-fsyntax-only because I don't have nvptx assembler. Enable -fsyntax-only
changes the order of the compilation somehow so I need to reorder the
errors and warnings a little bit.


https://reviews.llvm.org/D39502

Files:
  lib/Driver/Compilation.cpp
  test/Driver/cuda-bail-out.cu
  test/Driver/output-file-cleanup.c
  test/Driver/unix-conformance.c

Index: test/Driver/unix-conformance.c
===
--- /dev/null
+++ test/Driver/unix-conformance.c
@@ -0,0 +1,24 @@
+// Check UNIX conformance for cc/c89/c99
+// When c99 encounters a compilation error that causes an object file not to be
+// created, it shall write a diagnostic to standard error and continue to
+// compile other source code operands, but it shall not perform the link phase
+// and it shall return a non-zero exit status.
+
+// When given multiple .c files to compile, clang compiles them in order until
+// it hits an error, at which point it stops.
+//
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir
+// RUN: cd %t-dir
+//
+// RUN: touch %t-dir/1.c
+// RUN: echo "invalid C code" > %t-dir/2.c
+// RUN: touch %t-dir/3.c
+// RUN: echo "invalid C code" > %t-dir/4.c
+// RUN: touch %t-dir/5.c
+// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
+// RUN: test -f %t-dir/1.s
+// RUN: test ! -f %t-dir/2.s
+// RUN: test -f %t-dir/3.s
+// RUN: test ! -f %t-dir/4.s
+// RUN: test -f %t-dir/5.s
Index: test/Driver/output-file-cleanup.c
===
--- test/Driver/output-file-cleanup.c
+++ test/Driver/output-file-cleanup.c
@@ -41,18 +41,3 @@
 // RUN: not %clang -S %t-dir/1.c %t-dir/2.c
 // RUN: test -f %t-dir/1.s
 // RUN: test ! -f %t-dir/2.s
-
-// When given multiple .c files to compile, clang compiles them in order until
-// it hits an error, at which point it stops.
-//
-// RUN: touch %t-dir/1.c
-// RUN: echo "invalid C code" > %t-dir/2.c
-// RUN: touch %t-dir/3.c
-// RUN: echo "invalid C code" > %t-dir/4.c
-// RUN: touch %t-dir/5.c
-// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
-// RUN: test -f %t-dir/1.s
-// RUN: test ! -f %t-dir/2.s
-// RUN: test ! -f %t-dir/3.s
-// RUN: test ! -f %t-dir/4.s
-// RUN: test ! -f %t-dir/5.s
Index: test/Driver/cuda-bail-out.cu
===
--- /dev/null
+++ test/Driver/cuda-bail-out.cu
@@ -0,0 +1,47 @@
+// Test cland driver bailed out after one error during CUDA compilation
+
+// REQUIRES: clang-driver
+// REQUIRES: powerpc-registered-target
+// REQUIRES: nvptx-registered-target
+
+#if __CUDA_ARCH__ == 300
+#warning  No error during compilation for sm_30.
+#endif
+
+# if __CUDA_ARCH__ == 350
+#error Error during compilation for sm_35
+#endif
+
+# if __CUDA_ARCH__ == 600
+#error Error during compilation for sm_60
+#endif
+
+#ifndef __CUDA_ARCH__
+#ifdef HOST_FAIL
+#error Error during compilation for host
+#else
+#warning No error during compilation for host
+#endif
+#endif
+
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -fsyntax-only -nocudalib \
+// RUN:   -nocudainc --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: No error during compilation for host
+// CHECK: No error during compilation for sm_30.
+// CHECK: Error during compilation for sm_35
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -fsyntax-only -nocudalib \
+// RUN:   -nocudainc --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_60 %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-DEVICE-ERROR %s
+// CHECK-DEVICE-ERROR: No error during compilation for host
+// CHECK-DEVICE-ERROR: Error during compilation for sm_35
+// CHECK-DEVICE-ERROR-NOT: Error during compilation for sm_60
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -fsyntax-only \
+// RUN:   --cuda-gpu-arch=sm_35 -DHOST_FAIL -nocudalib -nocudainc %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-HOST-ERROR %s
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -182,16 +182,51 @@
   return ExecutionFailed ? 1 : Res;
 }
 
-void Compilation::ExecuteJobs(
-const JobList &Jobs,
-SmallVectorImpl> &FailingCommands) const {
+using FailingCommandList = SmallVectorImpl>;
+
+static bool ActionFailed(const Action *A,
+ const FailingCommandList &FailingCommands) {
+
+  if (FailingCommands.empty())
+return false;
+
+  // CUDA can have the same input source code compiled multiple times so do not
+  // compiled again if there are already failures. It is OK to abort the CUDA
+  // pipel

Re: [PATCH] D39622: Fix type debug information generation for enum-based template specialization

[David Blaikie via cfe-commits]

Ping \/

On Fri, Nov 3, 2017 at 4:22 PM David Blaikie  wrote:

> When you say "symbols table" - what are you referring to?
>
> On Fri, Nov 3, 2017 at 4:20 PM Adrian Prantl via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> aprantl added a comment.
>>
>> Can you add a testcase?
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D39622
>>
>>
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38708: [AST] Flag the typo-corrected nodes for better tooling

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D38708



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

[PATCH] D39707: [analyzer] assume bitwise arithmetic axioms

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun.

Patches the solver to assume that bitwise OR of an unsigned value with a 
constant always produces a value larger-or-equal than the constant, and bitwise 
AND with a constant always produces a value less-or-equal than the constant.

This patch is especially useful in the context of using bitwise arithmetic for 
error code encoding: the analyzer would be able to state that the error code 
produced using a bitwise OR is non-zero.

I have considered in very great detail the option of adding the matching code 
in ExprEngine / SimpleSValBuilder and have found it unfeasible: ideally we 
would like to pattern match in a great variety of situations: whether the 
created bitwise value is stored, compared, or added to other values.
Considering all those cases would have led to a very large amount of code 
duplication.


https://reviews.llvm.org/D39707

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constant-folding.c


Index: test/Analysis/constant-folding.c
===
--- test/Analysis/constant-folding.c
+++ test/Analysis/constant-folding.c
@@ -76,3 +76,18 @@
   clang_analyzer_eval(b >= a); // expected-warning{{TRUE}}
   clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
 }
+
+void testBitwiseRules(unsigned int a, int b) {
+  clang_analyzer_eval((a | 1) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 | a) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a & 1) < 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 & a) < 2); // expected-warning{{TRUE}}
+
+  unsigned int c = a;
+  c |= 1;
+  clang_analyzer_eval((c | 0) == 0); // expected-warning{{FALSE}}
+
+  // Rules don't apply to signed typed, as the values might be negative.
+  clang_analyzer_eval((b | 1) > 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((b | 1) == 0); // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -460,6 +460,44 @@
   return Changed ? State->set(CR) : State;
 }
 
+/// \brief Apply implicit constraints for bitwise OR- and AND-.
+/// For unsigned types, bitwise OR with a constant always returns
+/// a value greater-or-equal than the constant, and bitwise AND
+/// returns a value less-or-equal then the constant.
+///
+/// Pattern matches the expression \p Sym against those rule,
+/// and applies the required constraints.
+static RangeSet applyBitwiseConstraints(
+BasicValueFactory &BV,
+RangeSet::Factory &F,
+QualType T,
+RangeSet Result,
+SymbolRef Sym) {
+
+  if (const SymIntExpr *SIE = dyn_cast(Sym)) {
+
+// Only apply the rule to unsigned types, otherwise sign
+// may change and ordering will be violated.
+if (!SIE->getLHS()->getType()->isUnsignedIntegerType())
+  return Result;
+
+switch (SIE->getOpcode()) {
+  case BO_Or:
+
+// result >= constant
+return Result.Intersect(BV, F, SIE->getRHS(), BV.getMaxValue(T));
+  case BO_And:
+
+// result <= constant
+return Result.Intersect(BV, F, BV.getMinValue(T), SIE->getRHS());
+  default:
+break;
+}
+  }
+  return Result;
+
+}
+
 RangeSet RangeConstraintManager::getRange(ProgramStateRef State,
   SymbolRef Sym) {
   if (ConstraintRangeTy::data_type *V = State->get(Sym))
@@ -479,6 +517,7 @@
   --IntType.getZeroValue());
   }
 
+  Result = applyBitwiseConstraints(BV, F, T, Result, Sym);
   return Result;
 }
 


Index: test/Analysis/constant-folding.c
===
--- test/Analysis/constant-folding.c
+++ test/Analysis/constant-folding.c
@@ -76,3 +76,18 @@
   clang_analyzer_eval(b >= a); // expected-warning{{TRUE}}
   clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
 }
+
+void testBitwiseRules(unsigned int a, int b) {
+  clang_analyzer_eval((a | 1) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 | a) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a & 1) < 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 & a) < 2); // expected-warning{{TRUE}}
+
+  unsigned int c = a;
+  c |= 1;
+  clang_analyzer_eval((c | 0) == 0); // expected-warning{{FALSE}}
+
+  // Rules don't apply to signed typed, as the values might be negative.
+  clang_analyzer_eval((b | 1) > 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((b | 1) == 0); // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -460,6 +460,44 @@
   return Ch

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In https://reviews.llvm.org/D39502#917267, @steven_wu wrote:

> Improve testcase according to review feedback.
>
> In order to let compilation to be successful for one input, I need to use
>  -fsyntax-only because I don't have nvptx assembler. Enable -fsyntax-only
>  changes the order of the compilation somehow so I need to reorder the
>  errors and warnings a little bit.


Right.  With -fsyntax-only host compilation no longer depends on device-side 
binary. You may want to add a comment in the test explaining peculiar order of 
checks.




Comment at: test/Driver/cuda-bail-out.cu:47
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35

To make it more robust, I'd add another copy of the last line before 
CHECK-HOST-ERROR: so it would catch device-side errors if they were to happen 
ahead of the host.

Generally speaking, expected behavior is "I want to see an error from one 
compilation only". We don't really care which CUDA compilation phase produces 
it. Perhaps all we need in all test cases is:

```
CHECK: Error during compilation
CHECK-NOT:  Error during compilation
```

This way we don't need to depend on specific phase order.


https://reviews.llvm.org/D39502



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

r317532 - Clarify the error message for unsupported aliases on Darwin

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Nov  6 16:31:19 2017
New Revision: 317532

URL: http://llvm.org/viewvc/llvm-project?rev=317532&view=rev
Log:
Clarify the error message for unsupported aliases on Darwin

rdar://35109556

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/Sema/attr-alias.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=317532&r1=317531&r2=317532&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Nov  6 16:31:19 
2017
@@ -2778,7 +2778,7 @@ def err_attribute_weakref_not_global_con
 def err_attribute_weakref_without_alias : Error<
   "weakref declaration of %0 must also have an alias attribute">;
 def err_alias_not_supported_on_darwin : Error <
-  "only weak aliases are supported on darwin">;
+  "aliases are not supported on darwin">;
 def err_alias_to_undefined : Error<
   "%select{alias|ifunc}0 must point to a defined %select{variable or 
|}1function">;
 def warn_alias_to_weak_alias : Warning<

Modified: cfe/trunk/test/Sema/attr-alias.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-alias.c?rev=317532&r1=317531&r2=317532&view=diff
==
--- cfe/trunk/test/Sema/attr-alias.c (original)
+++ cfe/trunk/test/Sema/attr-alias.c Mon Nov  6 16:31:19 2017
@@ -2,7 +2,4 @@
 
 void g() {}
 
-// It is important that the following string be in the error message. The gcc
-// testsuite looks for it to decide if a target supports aliases.
-
-void f() __attribute__((alias("g"))); //expected-error {{only weak aliases are 
supported}}
+void f() __attribute__((alias("g"))); //expected-error {{aliases are not 
supported on darwin}}


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

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 121814.
george.karpenkov edited the summary of this revision.
george.karpenkov added a comment.

Addressed review comments.


https://reviews.llvm.org/D39584

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet &Dst) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+RightV = conjureOffsetSymbolOnLocation(
+RightV, LeftV, RHS, svalBuilder, Count, LCtx);
+LeftV = conjureOffsetSymbolOnLocation(
+LeftV, RightV, LHS, svalBuilder, Count, LCtx);
   }
 
   // Although we don't yet model pointers-to-members, we do need to make


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet &Dst) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+RightV = conjureOffsetSymbolO

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: test/Driver/cuda-bail-out.cu:47
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35

tra wrote:
> To make it more robust, I'd add another copy of the last line before 
> CHECK-HOST-ERROR: so it would catch device-side errors if they were to happen 
> ahead of the host.
> 
> Generally speaking, expected behavior is "I want to see an error from one 
> compilation only". We don't really care which CUDA compilation phase produces 
> it. Perhaps all we need in all test cases is:
> 
> ```
> CHECK: Error during compilation
> CHECK-NOT:  Error during compilation
> ```
> 
> This way we don't need to depend on specific phase order.
That will be a design choice for CUDA driver. I have no preference going either 
direction. Just let me know so I will update the test case.

Speaking of "-fsyntax-only", this is another interesting behavior of clang cuda 
driver:
```
$ clang -x cuda /dev/null -x c /dev/null -ccc-print-phases
14: input, "/dev/null", c, (host-cuda)
$ clang -fsyntax-only -x cuda /dev/null -x c /dev/null -ccc-print-phases
9: input, "/dev/null", c
```
So depending on if -fsyntax-only is used or not, the c language part can be 
either offloading or not offloading.
This is a corner case that the driver behavior will change after this patch.


https://reviews.llvm.org/D39502



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: test/Driver/cuda-bail-out.cu:47
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35

steven_wu wrote:
> tra wrote:
> > To make it more robust, I'd add another copy of the last line before 
> > CHECK-HOST-ERROR: so it would catch device-side errors if they were to 
> > happen ahead of the host.
> > 
> > Generally speaking, expected behavior is "I want to see an error from one 
> > compilation only". We don't really care which CUDA compilation phase 
> > produces it. Perhaps all we need in all test cases is:
> > 
> > ```
> > CHECK: Error during compilation
> > CHECK-NOT:  Error during compilation
> > ```
> > 
> > This way we don't need to depend on specific phase order.
> That will be a design choice for CUDA driver. I have no preference going 
> either direction. Just let me know so I will update the test case.
> 
> Speaking of "-fsyntax-only", this is another interesting behavior of clang 
> cuda driver:
> ```
> $ clang -x cuda /dev/null -x c /dev/null -ccc-print-phases
> 14: input, "/dev/null", c, (host-cuda)
> $ clang -fsyntax-only -x cuda /dev/null -x c /dev/null -ccc-print-phases
> 9: input, "/dev/null", c
> ```
> So depending on if -fsyntax-only is used or not, the c language part can be 
> either offloading or not offloading.
> This is a corner case that the driver behavior will change after this patch.
OK. Let's just check for one error only. 

As for the second, it is, IMO a problem. The file after ```-x c```  should have 
been treated as plain C input, regardless of -fsyntax-only.  It's reproducible 
in clean clang tree, so it's not due to this patch. 


https://reviews.llvm.org/D39502



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: test/Driver/cuda-bail-out.cu:47
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35

tra wrote:
> steven_wu wrote:
> > tra wrote:
> > > To make it more robust, I'd add another copy of the last line before 
> > > CHECK-HOST-ERROR: so it would catch device-side errors if they were to 
> > > happen ahead of the host.
> > > 
> > > Generally speaking, expected behavior is "I want to see an error from one 
> > > compilation only". We don't really care which CUDA compilation phase 
> > > produces it. Perhaps all we need in all test cases is:
> > > 
> > > ```
> > > CHECK: Error during compilation
> > > CHECK-NOT:  Error during compilation
> > > ```
> > > 
> > > This way we don't need to depend on specific phase order.
> > That will be a design choice for CUDA driver. I have no preference going 
> > either direction. Just let me know so I will update the test case.
> > 
> > Speaking of "-fsyntax-only", this is another interesting behavior of clang 
> > cuda driver:
> > ```
> > $ clang -x cuda /dev/null -x c /dev/null -ccc-print-phases
> > 14: input, "/dev/null", c, (host-cuda)
> > $ clang -fsyntax-only -x cuda /dev/null -x c /dev/null -ccc-print-phases
> > 9: input, "/dev/null", c
> > ```
> > So depending on if -fsyntax-only is used or not, the c language part can be 
> > either offloading or not offloading.
> > This is a corner case that the driver behavior will change after this patch.
> OK. Let's just check for one error only. 
> 
> As for the second, it is, IMO a problem. The file after ```-x c```  should 
> have been treated as plain C input, regardless of -fsyntax-only.  It's 
> reproducible in clean clang tree, so it's not due to this patch. 
The corner case I am talking about is after this patch, "-x c" will be syntax 
checked even if "-x cuda" failed the syntax check (which sounds good) but only 
when using "-fsyntax-only"

I am updating the test case.


https://reviews.llvm.org/D39502



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

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 121822.
steven_wu added a comment.

Update testcase for CUDA driver


https://reviews.llvm.org/D39502

Files:
  lib/Driver/Compilation.cpp
  test/Driver/cuda-bail-out.cu
  test/Driver/output-file-cleanup.c
  test/Driver/unix-conformance.c

Index: test/Driver/unix-conformance.c
===
--- /dev/null
+++ test/Driver/unix-conformance.c
@@ -0,0 +1,24 @@
+// Check UNIX conformance for cc/c89/c99
+// When c99 encounters a compilation error that causes an object file not to be
+// created, it shall write a diagnostic to standard error and continue to
+// compile other source code operands, but it shall not perform the link phase
+// and it shall return a non-zero exit status.
+
+// When given multiple .c files to compile, clang compiles them in order until
+// it hits an error, at which point it stops.
+//
+// RUN: rm -rf %t-dir
+// RUN: mkdir -p %t-dir
+// RUN: cd %t-dir
+//
+// RUN: touch %t-dir/1.c
+// RUN: echo "invalid C code" > %t-dir/2.c
+// RUN: touch %t-dir/3.c
+// RUN: echo "invalid C code" > %t-dir/4.c
+// RUN: touch %t-dir/5.c
+// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
+// RUN: test -f %t-dir/1.s
+// RUN: test ! -f %t-dir/2.s
+// RUN: test -f %t-dir/3.s
+// RUN: test ! -f %t-dir/4.s
+// RUN: test -f %t-dir/5.s
Index: test/Driver/output-file-cleanup.c
===
--- test/Driver/output-file-cleanup.c
+++ test/Driver/output-file-cleanup.c
@@ -41,18 +41,3 @@
 // RUN: not %clang -S %t-dir/1.c %t-dir/2.c
 // RUN: test -f %t-dir/1.s
 // RUN: test ! -f %t-dir/2.s
-
-// When given multiple .c files to compile, clang compiles them in order until
-// it hits an error, at which point it stops.
-//
-// RUN: touch %t-dir/1.c
-// RUN: echo "invalid C code" > %t-dir/2.c
-// RUN: touch %t-dir/3.c
-// RUN: echo "invalid C code" > %t-dir/4.c
-// RUN: touch %t-dir/5.c
-// RUN: not %clang -S %t-dir/1.c %t-dir/2.c %t-dir/3.c %t-dir/4.c %t-dir/5.c
-// RUN: test -f %t-dir/1.s
-// RUN: test ! -f %t-dir/2.s
-// RUN: test ! -f %t-dir/3.s
-// RUN: test ! -f %t-dir/4.s
-// RUN: test ! -f %t-dir/5.s
Index: test/Driver/cuda-bail-out.cu
===
--- /dev/null
+++ test/Driver/cuda-bail-out.cu
@@ -0,0 +1,16 @@
+// Test clang driver bails out after one error during CUDA compilation.
+
+// REQUIRES: clang-driver
+// REQUIRES: powerpc-registered-target
+// REQUIRES: nvptx-registered-target
+
+#error compilation failed
+
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -fsyntax-only -nocudalib \
+// RUN:   -nocudainc %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64le-ibm-linux-gnu -fsyntax-only -nocudalib \
+// RUN:   -nocudainc --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_60 %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: error: compilation failed
+// CHECK-NOT: error: compilation failed
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -182,16 +182,51 @@
   return ExecutionFailed ? 1 : Res;
 }
 
-void Compilation::ExecuteJobs(
-const JobList &Jobs,
-SmallVectorImpl> &FailingCommands) const {
+using FailingCommandList = SmallVectorImpl>;
+
+static bool ActionFailed(const Action *A,
+ const FailingCommandList &FailingCommands) {
+
+  if (FailingCommands.empty())
+return false;
+
+  // CUDA can have the same input source code compiled multiple times so do not
+  // compiled again if there are already failures. It is OK to abort the CUDA
+  // pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda))
+return true;
+
+  for (const auto &CI : FailingCommands)
+if (A == &(CI.second->getSource()))
+  return true;
+
+  for (const Action *AI : A->inputs())
+if (ActionFailed(AI, FailingCommands))
+  return true;
+
+  return false;
+}
+
+static bool InputsOk(const Command &C,
+ const FailingCommandList &FailingCommands) {
+  return !ActionFailed(&C.getSource(), FailingCommands);
+}
+
+void Compilation::ExecuteJobs(const JobList &Jobs,
+  FailingCommandList &FailingCommands) const {
+  // According to UNIX standard, driver need to continue compiling all the
+  // inputs on the command line even one of them failed.
+  // In all but CLMode, execute all the jobs unless the necessary inputs for the
+  // job is missing due to previous failures.
   for (const auto &Job : Jobs) {
+if (!InputsOk(Job, FailingCommands))
+  continue;
 const Command *FailingCommand = nullptr;
 if (int Res = ExecuteCommand(Job, FailingCommand)) {
   FailingCommands.push_back(std::make_pair(Res, FailingCommand));
-  // Bail as soon as one command fails, so we don't output duplicate error
-  // messages if we die on e.g. the same file.
-  return;
+  // Bail as soo

[PATCH] D39708: [OMPT] Fix test cancel_parallel.c

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.

If a parallel region is cancelled, execution resumes at the end
of the structured block. That is why this test cannot use the
"normal" macros that print right after inserting the label.
Instead it previously printed the addresses before the pragma
and swapped the checks compared to the other tests.

However, this does not work because FileChecks '*' is greedy
so that RETURN_ADDRESS always matched the second address. This
makes the test fail when an "overflow" occurrs and the first
address matches the value of codeptr_ra.

I discovered this on my MacBook but I'm unable to reproduce the
failure with the current version. Nevertheless we should fix this
problem to avoid that this test fails later after an unrelated change.


https://reviews.llvm.org/D39708

Files:
  runtime/test/ompt/cancel/cancel_parallel.c


Index: runtime/test/ompt/cancel/cancel_parallel.c
===
--- runtime/test/ompt/cancel/cancel_parallel.c
+++ runtime/test/ompt/cancel/cancel_parallel.c
@@ -6,37 +6,48 @@
 #include "callback.h"
 #include "omp.h"

-int main()
-{
+int main() {
+  // We have to save the addresses because the compiler outlines the parallel
+  // regions and labels are only valid within one function.
+  void *master, *worker;
   #pragma omp parallel num_threads(2)
   {
-if(omp_get_thread_num() == 0)
-{
-  printf("%" PRIu64 ": fuzzy_address=0x%lx or 0x%lx\n", 
ompt_get_thread_data()->value, ((uint64_t)(char*)(&& ompt_label_1))/256-1, 
((uint64_t)(char*)(&& ompt_label_1))/256);
+if (omp_get_thread_num() == 0) {
+  master = get_ompt_label_address(1);
   #pragma omp cancel parallel
-  print_fuzzy_address(1); //does not actually print the address but 
provides a label
-}
-else
-{
+  define_ompt_label(1);
+  // We cannot print at this location because the parallel region is 
cancelled!
+} else {
   delay(100);
-  printf("%" PRIu64 ": fuzzy_address=0x%lx or 0x%lx\n", 
ompt_get_thread_data()->value, ((uint64_t)(char*)(&& ompt_label_2))/256-1, 
((uint64_t)(char*)(&& ompt_label_2))/256);
+  worker = get_ompt_label_address(2);
   #pragma omp cancellation point parallel
-  print_fuzzy_address(2); //does not actually print the address but 
provides a label
+  define_ompt_label(2);
+  // We cannot print at this location because the parallel region is 
cancelled!
+}
+  }
+
+  // We will get the same threads from libomp.
+  #pragma omp parallel num_threads(2)
+  {
+if (omp_get_thread_num() == 0) {
+  print_fuzzy_address_blocks(master);
+} else {
+  print_fuzzy_address_blocks(worker);
 }
   }


   // Check if libomp supports the callbacks for this test.
   // CHECK-NOT: {{^}}0: Could not register callback 'ompt_callback_task_create'
   // CHECK-NOT: {{^}}0: Could not register callback 'ompt_callback_cancel'

   // CHECK: {{^}}0: NULL_POINTER=[[NULL:.*$]]
   // CHECK: {{^}}[[MASTER_ID:[0-9]+]]: ompt_event_task_create: 
parent_task_id=[[PARENT_TASK_ID:[0-9]+]], parent_task_frame.exit=[[NULL]], 
parent_task_frame.reenter=[[NULL]], new_task_id=[[TASK_ID:[0-9]+]], 
codeptr_ra=[[NULL]], task_type=ompt_task_initial=1, has_dependences=no
-  // CHECK: {{^}}[[MASTER_ID]]: fuzzy_address={{.*}}[[RETURN_ADDRESS:0x[0-f]+]]
-  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_cancel: 
task_data=[[TASK_ID:[0-9]+]], 
flags=ompt_cancel_parallel|ompt_cancel_activated=17, 
codeptr_ra=[[RETURN_ADDRESS]]{{[0-f][0-f]}}
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_cancel: 
task_data=[[TASK_ID:[0-9]+]], 
flags=ompt_cancel_parallel|ompt_cancel_activated=17, 
codeptr_ra=[[RETURN_ADDRESS:0x[0-f]+]]{{[0-f][0-f]}}
+  // CHECK: {{^}}[[MASTER_ID]]: fuzzy_address={{.*}}[[RETURN_ADDRESS]]

-  // CHECK: {{^}}[[THREAD_ID:[0-9]+]]: 
fuzzy_address={{.*}}[[OTHER_RETURN_ADDRESS:0x[0-f]+]]
-  // CHECK: {{^}}[[THREAD_ID]]: ompt_event_cancel: 
task_data=[[TASK_ID:[0-9]+]], 
flags=ompt_cancel_parallel|ompt_cancel_detected=33, 
codeptr_ra=[[OTHER_RETURN_ADDRESS]]{{[0-f][0-f]}}
+  // CHECK: {{^}}[[THREAD_ID:[0-9]+]]: ompt_event_cancel: 
task_data=[[TASK_ID:[0-9]+]], 
flags=ompt_cancel_parallel|ompt_cancel_detected=33, 
codeptr_ra=[[OTHER_RETURN_ADDRESS:0x[0-f]+]]{{[0-f][0-f]}}
+  // CHECK: {{^}}[[THREAD_ID]]: fuzzy_address={{.*}}[[OTHER_RETURN_ADDRESS]]

   return 0;
 }


Index: runtime/test/ompt/cancel/cancel_parallel.c
===
--- runtime/test/ompt/cancel/cancel_parallel.c
+++ runtime/test/ompt/cancel/cancel_parallel.c
@@ -6,37 +6,48 @@
 #include "callback.h"
 #include "omp.h"

-int main()
-{
+int main() {
+  // We have to save the addresses because the compiler outlines the parallel
+  // regions and labels are only valid within one function.
+  void *master, *worker;
   #pragma omp parallel num_threads(2)
   {
-if(omp_get_thread_num() == 0)
-{
-  printf("%" PRIu64 ": fuzzy_address=0x%lx or 0x%lx\n", ompt_get_thread_data()->value, 

[PATCH] D39579: BuiltinOperatorOverloadBuilder: Don't consider types that are unavailable on the target (PR35174)

[Hans Wennborg via Phabricator via cfe-commits]

hans marked 2 inline comments as done.
hans added inline comments.



Comment at: test/CodeGenCXX/microsoft-cannot-mangle-float128.cpp:3-11
+template  struct B;
+template  constexpr bool is_floating_point_v = false;
+
+struct StrictNumeric {
+  StrictNumeric(int);
+  template > = nullptr> operator 
Dst();
+};

rsmith wrote:
> This seems like a fragile way of checking for the bug; I'd prefer a test that 
> tests `Sema` rather than code generation. For instance, how about a class 
> with a conversion operator that converts to `T` with an `enable_if` disabling 
> it for `float`, `double`, and `long double`, and a check that an instance of 
> such a class can't be used in floating-point arithmetic?
Thanks! That sounds like a good plan.


https://reviews.llvm.org/D39579



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

[PATCH] D39579: BuiltinOperatorOverloadBuilder: Don't consider types that are unavailable on the target (PR35174)

[Hans Wennborg via Phabricator via cfe-commits]

hans updated this revision to Diff 121824.
hans edited the summary of this revision.
hans added a comment.

Addressing comments and changing the test to exercise Sema more directly.

Please take another look.


https://reviews.llvm.org/D39579

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/microsoft-vs-float128.cpp

Index: test/SemaCXX/microsoft-vs-float128.cpp
===
--- /dev/null
+++ test/SemaCXX/microsoft-vs-float128.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fms-compatibility -fms-extensions -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-compatibility -fms-extensions -fsyntax-only -verify -std=c++11 -DMS %s
+
+template  struct enable_if {};
+template<> struct enable_if { typedef void type; };
+
+template  struct is_same { static constexpr bool value = false; };
+template  struct is_same { static constexpr bool value = true; };
+
+
+
+
+struct S {
+  // The only numeric types S can be converted to is __int128 and __float128.
+  template ::value ||
+  is_same::value ||
+  is_same::value)>::type>
+  operator T() { return T(); }
+};
+
+void f() {
+#ifdef MS
+  // When targeting Win32, __float128 and __int128 do not exist, so the S
+  // object cannot be converted to anything usable in the expression.
+  // expected-error@+2{{invalid operands to binary expression ('S' and 'double')}}
+#endif
+  double d = S() + 1.0;
+#ifndef MS
+  // expected-error@-2{{use of overloaded operator '+' is ambiguous}}
+  // expected-note@-3 36{{built-in candidate operator+}}
+#endif
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7615,53 +7615,58 @@
   SmallVectorImpl &CandidateTypes;
   OverloadCandidateSet &CandidateSet;
 
-  // Define some constants used to index and iterate over the arithemetic types
-  // provided via the getArithmeticType() method below.
-  // The "promoted arithmetic types" are the arithmetic
+  SmallVector ArithmeticTypes;
+
+  // Define some indices used to iterate over the arithemetic types in
+  // ArithmeticTypes.  The "promoted arithmetic types" are the arithmetic
   // types are that preserved by promotion (C++ [over.built]p2).
-  static const unsigned FirstIntegralType = 4;
-  static const unsigned LastIntegralType = 21;
-  static const unsigned FirstPromotedIntegralType = 4,
-LastPromotedIntegralType = 12;
-  static const unsigned FirstPromotedArithmeticType = 0,
-LastPromotedArithmeticType = 12;
-  static const unsigned NumArithmeticTypes = 21;
-
-  /// \brief Get the canonical type for a given arithmetic type index.
-  CanQualType getArithmeticType(unsigned index) {
-assert(index < NumArithmeticTypes);
-static CanQualType ASTContext::* const
-  ArithmeticTypes[NumArithmeticTypes] = {
-  // Start of promoted types.
-  &ASTContext::FloatTy,
-  &ASTContext::DoubleTy,
-  &ASTContext::LongDoubleTy,
-  &ASTContext::Float128Ty,
-
-  // Start of integral types.
-  &ASTContext::IntTy,
-  &ASTContext::LongTy,
-  &ASTContext::LongLongTy,
-  &ASTContext::Int128Ty,
-  &ASTContext::UnsignedIntTy,
-  &ASTContext::UnsignedLongTy,
-  &ASTContext::UnsignedLongLongTy,
-  &ASTContext::UnsignedInt128Ty,
-  // End of promoted types.
-
-  &ASTContext::BoolTy,
-  &ASTContext::CharTy,
-  &ASTContext::WCharTy,
-  &ASTContext::Char16Ty,
-  &ASTContext::Char32Ty,
-  &ASTContext::SignedCharTy,
-  &ASTContext::ShortTy,
-  &ASTContext::UnsignedCharTy,
-  &ASTContext::UnsignedShortTy,
-  // End of integral types.
-  // FIXME: What about complex? What about half?
-};
-return S.Context.*ArithmeticTypes[index];
+  unsigned FirstIntegralType,
+   LastIntegralType;
+  unsigned FirstPromotedIntegralType,
+   LastPromotedIntegralType;
+  unsigned FirstPromotedArithmeticType,
+   LastPromotedArithmeticType;
+  unsigned NumArithmeticTypes;
+
+  void InitArithmeticTypes() {
+// Start of promoted types.
+FirstPromotedArithmeticType = 0;
+ArithmeticTypes.push_back(S.Context.FloatTy);
+ArithmeticTypes.push_back(S.Context.DoubleTy);
+ArithmeticTypes.push_back(S.Context.LongDoubleTy);
+if (S.Context.getTargetInfo().hasFloat128Type())
+  ArithmeticTypes.push_back(S.Context.Float128Ty);
+
+// Start of integral types.
+FirstIntegralType = ArithmeticTypes.size();
+FirstPromotedIntegralType = ArithmeticTypes.size();
+ArithmeticTypes.push_back(S.Context.IntTy);
+ArithmeticTypes.push_back(S.Context.LongTy);
+ArithmeticTypes.push_back(S.Context.LongLongTy);
+if (S.Context.getTargetInfo().hasInt128Type())
+  ArithmeticTypes.push_back(S.Context.Int128Ty);
+ArithmeticTypes.push_back(S.Context.Unsigned

[PATCH] D39707: [analyzer] assume bitwise arithmetic axioms

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 121825.

https://reviews.llvm.org/D39707

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constant-folding.c


Index: test/Analysis/constant-folding.c
===
--- test/Analysis/constant-folding.c
+++ test/Analysis/constant-folding.c
@@ -76,3 +76,23 @@
   clang_analyzer_eval(b >= a); // expected-warning{{TRUE}}
   clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
 }
+
+void testBitwiseRules(unsigned int a, int b) {
+  clang_analyzer_eval((a | 1) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 | a) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a & 1) < 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 & a) < 2); // expected-warning{{TRUE}}
+
+  unsigned int c = a;
+  c |= 1;
+  clang_analyzer_eval((c | 0) == 0); // expected-warning{{FALSE}}
+
+  // Rules don't apply to signed typed, as the values might be negative.
+  clang_analyzer_eval((b | 1) > 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((b | 1) == 0); // expected-warning{{UNKNOWN}}
+
+  // Check that dynamically computed constants also work.
+  int constant = 1 << 3;
+  unsigned int d = a | constant;
+  clang_analyzer_eval(constant > 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -460,6 +460,44 @@
   return Changed ? State->set(CR) : State;
 }
 
+/// \brief Apply implicit constraints for bitwise OR- and AND-.
+/// For unsigned types, bitwise OR with a constant always returns
+/// a value greater-or-equal than the constant, and bitwise AND
+/// returns a value less-or-equal then the constant.
+///
+/// Pattern matches the expression \p Sym against those rule,
+/// and applies the required constraints.
+static RangeSet applyBitwiseConstraints(
+BasicValueFactory &BV,
+RangeSet::Factory &F,
+QualType T,
+RangeSet Result,
+SymbolRef Sym) {
+
+  if (const SymIntExpr *SIE = dyn_cast(Sym)) {
+
+// Only apply the rule to unsigned types, otherwise sign
+// may change and ordering will be violated.
+if (!SIE->getLHS()->getType()->isUnsignedIntegerType())
+  return Result;
+
+switch (SIE->getOpcode()) {
+  case BO_Or:
+
+// result >= constant
+return Result.Intersect(BV, F, SIE->getRHS(), BV.getMaxValue(T));
+  case BO_And:
+
+// result <= constant
+return Result.Intersect(BV, F, BV.getMinValue(T), SIE->getRHS());
+  default:
+break;
+}
+  }
+  return Result;
+
+}
+
 RangeSet RangeConstraintManager::getRange(ProgramStateRef State,
   SymbolRef Sym) {
   if (ConstraintRangeTy::data_type *V = State->get(Sym))
@@ -479,6 +517,7 @@
   --IntType.getZeroValue());
   }
 
+  Result = applyBitwiseConstraints(BV, F, T, Result, Sym);
   return Result;
 }
 


Index: test/Analysis/constant-folding.c
===
--- test/Analysis/constant-folding.c
+++ test/Analysis/constant-folding.c
@@ -76,3 +76,23 @@
   clang_analyzer_eval(b >= a); // expected-warning{{TRUE}}
   clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
 }
+
+void testBitwiseRules(unsigned int a, int b) {
+  clang_analyzer_eval((a | 1) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 | a) > 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a & 1) < 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval((1 & a) < 2); // expected-warning{{TRUE}}
+
+  unsigned int c = a;
+  c |= 1;
+  clang_analyzer_eval((c | 0) == 0); // expected-warning{{FALSE}}
+
+  // Rules don't apply to signed typed, as the values might be negative.
+  clang_analyzer_eval((b | 1) > 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((b | 1) == 0); // expected-warning{{UNKNOWN}}
+
+  // Check that dynamically computed constants also work.
+  int constant = 1 << 3;
+  unsigned int d = a | constant;
+  clang_analyzer_eval(constant > 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -460,6 +460,44 @@
   return Changed ? State->set(CR) : State;
 }
 
+/// \brief Apply implicit constraints for bitwise OR- and AND-.
+/// For unsigned types, bitwise OR with a constant always returns
+/// a value greater-or-equal than the constant, and bitwise AND
+/// returns a value less-or-equal then the constant.
+///
+/// Pattern matches the expression \p Sym against those rule,
+/// and applies the required constraints.
+static RangeSet applyBitwiseConstraints(
+BasicValueFactory &BV,
+Rang

[PATCH] D39709: [analyzer] [NFC] remove duplicated function

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun.

Two copies of `getSymLERange` in `RangeConstraintManager` are virtually 
identical, which is clearly bad.
This patch uses lambdas to call one from another (assuming that we would like 
to avoid getting ranges from the state when necessary).

If the latter is not even the concern, it would be even easier to do the 
computation eagerly, and then pass `RangeSet` directly instead of a lambda.


https://reviews.llvm.org/D39709

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp


Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -354,7 +354,8 @@
   RangeSet getSymLERange(ProgramStateRef St, SymbolRef Sym,
  const llvm::APSInt &Int,
  const llvm::APSInt &Adjustment);
-  RangeSet getSymLERange(const RangeSet &RS, const llvm::APSInt &Int,
+  RangeSet getSymLERange(llvm::function_ref RS,
+ const llvm::APSInt &Int,
  const llvm::APSInt &Adjustment);
   RangeSet getSymGERange(ProgramStateRef St, SymbolRef Sym,
  const llvm::APSInt &Int,
@@ -676,59 +677,39 @@
   return New.isEmpty() ? nullptr : St->set(Sym, New);
 }
 
-RangeSet RangeConstraintManager::getSymLERange(const RangeSet &RS,
-   const llvm::APSInt &Int,
-   const llvm::APSInt &Adjustment) 
{
+RangeSet RangeConstraintManager::getSymLERange(
+  llvm::function_ref RS,
+  const llvm::APSInt &Int,
+  const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
   switch (AdjustmentType.testInRange(Int, true)) {
   case APSIntType::RTR_Below:
 return F.getEmptySet();
   case APSIntType::RTR_Within:
 break;
   case APSIntType::RTR_Above:
-return RS;
+return RS();
   }
 
   // Special case for Int == Max. This is always feasible.
   llvm::APSInt ComparisonVal = AdjustmentType.convert(Int);
   llvm::APSInt Max = AdjustmentType.getMaxValue();
   if (ComparisonVal == Max)
-return RS;
+return RS();
 
   llvm::APSInt Min = AdjustmentType.getMinValue();
   llvm::APSInt Lower = Min - Adjustment;
   llvm::APSInt Upper = ComparisonVal - Adjustment;
 
-  return RS.Intersect(getBasicVals(), F, Lower, Upper);
+  return RS().Intersect(getBasicVals(), F, Lower, Upper);
 }
 
 RangeSet RangeConstraintManager::getSymLERange(ProgramStateRef St,
SymbolRef Sym,
const llvm::APSInt &Int,
const llvm::APSInt &Adjustment) 
{
-  // Before we do any real work, see if the value can even show up.
-  APSIntType AdjustmentType(Adjustment);
-  switch (AdjustmentType.testInRange(Int, true)) {
-  case APSIntType::RTR_Below:
-return F.getEmptySet();
-  case APSIntType::RTR_Within:
-break;
-  case APSIntType::RTR_Above:
-return getRange(St, Sym);
-  }
-
-  // Special case for Int == Max. This is always feasible.
-  llvm::APSInt ComparisonVal = AdjustmentType.convert(Int);
-  llvm::APSInt Max = AdjustmentType.getMaxValue();
-  if (ComparisonVal == Max)
-return getRange(St, Sym);
-
-  llvm::APSInt Min = AdjustmentType.getMinValue();
-  llvm::APSInt Lower = Min - Adjustment;
-  llvm::APSInt Upper = ComparisonVal - Adjustment;
-
-  return getRange(St, Sym).Intersect(getBasicVals(), F, Lower, Upper);
+  return getSymLERange([&] { return getRange(St, Sym); }, Int, Adjustment);
 }
 
 ProgramStateRef
@@ -745,8 +726,8 @@
   RangeSet New = getSymGERange(State, Sym, From, Adjustment);
   if (New.isEmpty())
 return nullptr;
-  New = getSymLERange(New, To, Adjustment);
-  return New.isEmpty() ? nullptr : State->set(Sym, New);
+  RangeSet Out = getSymLERange([&] { return New; }, To, Adjustment);
+  return Out.isEmpty() ? nullptr : State->set(Sym, Out);
 }
 
 ProgramStateRef RangeConstraintManager::assumeSymOutsideInclusiveRange(


Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -354,7 +354,8 @@
   RangeSet getSymLERange(ProgramStateRef St, SymbolRef Sym,
  const llvm::APSInt &Int,
  const llvm::APSInt &Adjustment);
-  RangeSet getSymLERange(const RangeSet &RS, const llvm::APSInt &Int,
+  RangeSet getSymLERange(llvm::function_ref RS,
+ const llvm::APSInt &Int,
  const llvm::APSInt &Adjustment);
   RangeSet getSymGERange(ProgramStateRef St, SymbolRef Sym,
   

r317537 - [analyzer] [NFC] Remove unused typedef from SVals.h

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Nov  6 18:02:10 2017
New Revision: 317537

URL: http://llvm.org/viewvc/llvm-project?rev=317537&view=rev
Log:
[analyzer] [NFC] Remove unused typedef from SVals.h

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=317537&r1=317536&r2=317537&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Mon Nov  
6 18:02:10 2017
@@ -103,9 +103,6 @@ public:
 return *static_cast(this);
   }
 
-  /// BufferTy - A temporary buffer to hold a set of SVals.
-  typedef SmallVector BufferTy;
-
   inline unsigned getRawKind() const { return Kind; }
   inline BaseKind getBaseKind() const { return (BaseKind) (Kind & BaseMask); }
   inline unsigned getSubKind() const { return (Kind & ~BaseMask) >> BaseBits; }


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

[PATCH] D39620: [analyzer] [NFC] Remove unused typedef

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL317537: [analyzer] [NFC] Remove unused typedef from SVals.h 
(authored by george.karpenkov).

Changed prior to commit:
  https://reviews.llvm.org/D39620?vs=121556&id=121827#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39620

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h


Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -103,9 +103,6 @@
 return *static_cast(this);
   }
 
-  /// BufferTy - A temporary buffer to hold a set of SVals.
-  typedef SmallVector BufferTy;
-
   inline unsigned getRawKind() const { return Kind; }
   inline BaseKind getBaseKind() const { return (BaseKind) (Kind & BaseMask); }
   inline unsigned getSubKind() const { return (Kind & ~BaseMask) >> BaseBits; }


Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -103,9 +103,6 @@
 return *static_cast(this);
   }
 
-  /// BufferTy - A temporary buffer to hold a set of SVals.
-  typedef SmallVector BufferTy;
-
   inline unsigned getRawKind() const { return Kind; }
   inline BaseKind getBaseKind() const { return (BaseKind) (Kind & BaseMask); }
   inline unsigned getSubKind() const { return (Kind & ~BaseMask) >> BaseBits; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r317538 - ClangdTests/JSONExprTests.cpp: Appease g++-4.8 to move raw string literal out of macro arg.

[NAKAMURA Takumi via cfe-commits]

Author: chapuni
Date: Mon Nov  6 18:18:24 2017
New Revision: 317538

URL: http://llvm.org/viewvc/llvm-project?rev=317538&view=rev
Log:
ClangdTests/JSONExprTests.cpp: Appease g++-4.8 to move raw string literal out 
of macro arg.

Modified:
clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp?rev=317538&r1=317537&r2=317538&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/JSONExprTests.cpp Mon Nov  6 
18:18:24 2017
@@ -76,8 +76,7 @@ TEST(JSONExprTests, Escaping) {
 }
 
 TEST(JSONExprTests, PrettyPrinting) {
-  EXPECT_EQ(
-  R"({
+  const char str[] = R"({
   "empty_array": [],
   "empty_object": {},
   "full_array": [
@@ -91,7 +90,10 @@ TEST(JSONExprTests, PrettyPrinting) {
   }
 ]
   }
-})",
+})";
+
+  EXPECT_EQ(
+  str,
   sp(obj{
   {"empty_object", obj{}},
   {"empty_array", {}},


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

[PATCH] D39537: Rename identifiers named `__output`

[Eric Fiselier via Phabricator via cfe-commits]

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

LGTM minus inline comments.




Comment at: test/support/nasty_macros.hpp:55
 
+#define __output NASTY_MACRO
+#define __input NASTY_MACRO

Shouldn't these not be defined when running the tests with a CHER compiler? 
Otherwise the macro will conflict with the keyword?


Repository:
  rL LLVM

https://reviews.llvm.org/D39537



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

[PATCH] D39080: [libcxx] [test] Tolerate even more [[nodiscard]] in the STL.

[Eric Fiselier via Phabricator via cfe-commits]

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

I think I would prefer if this patch "left an explicit trace" as to the reasons 
for all the `(void)` casts. Specifically I think a macro like 
`TEST_IGNORE_DISCARD ` might be nice.

@BillyONeal What do you think?

Otherwise this LGTM.


https://reviews.llvm.org/D39080



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

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

So, that change makes this very interesting, because I think the right way of 
looking at it is as the first in a larger family of warnings that attempt to 
treat typedefs as if they were a much stronger type-system feature, i.e. that 
warn about all sorts of conversions between different typedef types.  That 
should be good enough to serve as a basic rule for a stronger portability 
warning, as well as generally pointing out all sorts of potential logical 
errors like passing a bit_offset_t off as a byte_offset_t.

Such a warning really needs more exceptions than a simple exact-type-spelling 
rule would give you.  There are several language features that add type sugar 
which should really be ignored for the purposes of the warning, such as typeof 
and decltype; and conversely, there are several features that remove (or just 
never add) type sugar that also shouldn't cause problems, like literals or C++ 
templates.

I think that feature could be really useful as a major new diagnostic, but I do 
want to warn you that it's probably a pretty large project, somewhat on the 
scale of implementing -Wconversion in the first place.  Also, yeah, my first 
thought is that it's probably outside of a reasonable rubric for even -Wextra, 
especially while it's being actively developed.


Repository:
  rL LLVM

https://reviews.llvm.org/D39462



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

[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin created this revision.
Herald added a subscriber: szepet.

The ObjCGenerics checker warns on a cast when there is no subtyping relationship
between the tracked type of the value and the destination type of the cast. It
does this even if the cast was explicitly written. This means the user can't
write an explicit cast to silence the diagnostic.

This patch changes diagnostic emission to allow an explicit cast to silence
the diagnostic.

rdar://problem/33603303


https://reviews.llvm.org/D39711

Files:
  lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  test/Analysis/generics.m

Index: test/Analysis/generics.m
===
--- test/Analysis/generics.m
+++ test/Analysis/generics.m
@@ -247,6 +247,11 @@
   withMutArrMutableString(b); // expected-warning {{Conversion}}
 }
 
+void trustExplicitCastsAfterInference(MutableArray *a) {
+  withMutArrString(a);
+  withMutArrMutableString((MutableArray *)a); // no-warning
+}
+
 NSArray *getStrings();
 void enforceDynamicRulesInsteadOfStatic(NSArray *a) {
   NSArray *b = a;
@@ -4616,25 +4621,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line259
+// CHECK-NEXT:line264
 // CHECK-NEXT:col3
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line259
+// CHECK-NEXT:line264
 // CHECK-NEXT:col9
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:  
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col3
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col10
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -4650,25 +4655,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col3
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col10
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:  
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col19
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col19
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -4680,20 +4685,20 @@
 // CHECK-NEXT:  kindevent
 // CHECK-NEXT:  location
 // CHECK-NEXT:  
-// CHECK-NEXT:   line260
+// CHECK-NEXT:   line265
 // CHECK-NEXT:   col19
 // CHECK-NEXT:   file0
 // CHECK-NEXT:  
 // CHECK-NEXT:  ranges
 // CHECK-NEXT:  
 // CHECK-NEXT:
 // CHECK-NEXT: 
-// CHECK-NEXT:  line260
+// CHECK-NEXT:  line265
 // CHECK-NEXT:  col19
 // CHECK-NEXT:  file0
 // CHECK-NEXT: 
 // CHECK-NEXT: 
-// CHECK-NEXT:  line260
+// CHECK-NEXT:  line265
 // CHECK-NEXT:  col38
 // CHECK-NEXT:  file0
 // CHECK-NEXT: 
@@ -4709,20 +4714,20 @@
 // CHECK-NEXT:  kindevent
 // CHECK-NEXT:  location
 // CHECK-NEXT:  
-// CHECK-NEXT:   line260
+// CHECK-NEXT:   line265
 // CHECK-NEXT:   col19
 // CHECK-NEXT:   file0
 // CHECK-NEXT:  
 // CHECK-NEXT:  ranges
 // CHECK-NEXT:  
 // CHECK-NEXT:
 // CHECK-NEXT: 
-// CHECK-NEXT:  line260
+// CHECK-NEXT:  line265
 // CHECK-NEXT:  col19
 // CHECK-NEXT:  file0
 // CHECK-NEXT: 
 // CHECK-NEXT: 
-// CHECK-NEXT:  line260
+// CHECK-NEXT:  line265
 // CHECK-NEXT:  col38
 // CHECK-NEXT:  file0
 // CHECK-NEXT: 
@@ -4746,7 +4751,7 @@
 // CHECK-NEXT:   issue_hash_function_offset2
 // CHECK-NEXT:   location
 // CHECK-NEXT:   
-// CHECK-NEXT:line260
+// CHECK-NEXT:line265
 // CHECK-NEXT:col19
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -4762,25 +4767,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line259
+// CHECK-NEXT:line264
 // CHECK-NEXT:col3
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line259
+// CHECK-NEXT:line264
 // CHECK-NEXT:col9
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:  

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D39627#917175, @efriedma wrote:

> Adding cfe-commits.  (In the future, please make sure to CC that list instead 
> of llvm-commits for clang changes.)
>
> > I'm missing some background lingo here. What is the meaning of "a 
> > declaration" and "a definition" here?
>
> In most cases, we only emit the vtable for a class in one object file.  
> Informally, we can call that the "definition" of the vtable, as an analogy to 
> variables and functions.  See also 
> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable .


LLVM also distinguishes between a declaration (a Function or GlobalVariable 
which is presumed to be defined in a different Module) vs. a definition.


Repository:
  rL LLVM

https://reviews.llvm.org/D39627



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

[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

@xazax.hun Would you be willing to take a look?


https://reviews.llvm.org/D39711



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