[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154
+  if (const auto *MemberPointerT = Type->getAs()) {
+auto Pos = UserWrittenType.find("::");
+if (Pos != std::string::npos) { // might be hidden behind typedef etc.

Can this be confused by comments that contain `::`?



Comment at: test/clang-tidy/readability-one-name-per-declaration-simple.cpp:17
+long ** lint1, lint2 = 0, * lint3, **linn;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration statement can be 
split up into single line declarations [readability-one-name-per-declaration]
+// CHECK-FIXES: {{^}}long ** lint1;

You don't need to repeat the whole warning every time.


https://reviews.llvm.org/D27621



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

[PATCH] D27520: [clang-tidy] Add check for redundant function pointer dereferences

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289524: [clang-tidy] Add check for redundant function 
pointer dereferences (authored by malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D27520?vs=80642&id=81190#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27520

Files:
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-function-ptr-dereference.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-function-ptr-dereference.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -24,6 +24,7 @@
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
+#include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
@@ -59,6 +60,8 @@
 "readability-inconsistent-declaration-parameter-name");
 CheckFactories.registerCheck(
 "readability-misplaced-array-index");
+CheckFactories.registerCheck(
+"readability-redundant-function-ptr-dereference");
 CheckFactories.registerCheck(
 "readability-redundant-member-init");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
@@ -0,0 +1,37 @@
+//===--- RedundantFunctionPtrDereferenceCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantFunctionPtrDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantFunctionPtrDereferenceCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(unaryOperator(hasOperatorName("*"),
+   has(implicitCastExpr(
+   hasCastKind(CK_FunctionToPointerDecay
+ .bind("op"),
+ this);
+}
+
+void RedundantFunctionPtrDereferenceCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Operator = Result.Nodes.getNodeAs("op");
+  diag(Operator->getOperatorLoc(),
+   "redundant repeated dereference of function pointer")
+  << FixItHint::CreateRemoval(Operator->getOperatorLoc());
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantFunctionPtrDereferenceCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_FUNCTION_PTR_DEREFERENCE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_FUNCTION_PTR_DEREFERENCE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Eliminate redundant dereferences of a function pointer.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-function-ptr-dereference.html
+class RedundantFunctionPtrDereferenceCheck : public ClangTidyCheck {
+public:
+  RedundantFunctionPtrDereferenceCheck(StringRef Name, ClangTidyContext *

[clang-tools-extra] r289524 - [clang-tidy] Add check for redundant function pointer dereferences

[Malcolm Parsons via cfe-commits]

Author: malcolm.parsons
Date: Tue Dec 13 02:04:11 2016
New Revision: 289524

URL: http://llvm.org/viewvc/llvm-project?rev=289524&view=rev
Log:
[clang-tidy] Add check for redundant function pointer dereferences

Reviewers: alexfh, aaron.ballman, hokein

Subscribers: mgorny, JDevlieghere, cfe-commits

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

Added:

clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp

clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-function-ptr-dereference.rst

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-function-ptr-dereference.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=289524&r1=289523&r2=289524&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Tue Dec 13 
02:04:11 2016
@@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityMo
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
+  RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp?rev=289524&r1=289523&r2=289524&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp 
Tue Dec 13 02:04:11 2016
@@ -24,6 +24,7 @@
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
+#include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
@@ -59,6 +60,8 @@ public:
 "readability-inconsistent-declaration-parameter-name");
 CheckFactories.registerCheck(
 "readability-misplaced-array-index");
+CheckFactories.registerCheck(
+"readability-redundant-function-ptr-dereference");
 CheckFactories.registerCheck(
 "readability-redundant-member-init");
 CheckFactories.registerCheck(

Added: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp?rev=289524&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.cpp
 Tue Dec 13 02:04:11 2016
@@ -0,0 +1,37 @@
+//===--- RedundantFunctionPtrDereferenceCheck.cpp - 
clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantFunctionPtrDereferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantFunctionPtrDereferenceCheck::registerMatchers(MatchFinder 
*Finder) {
+  Finder->addMatcher(unaryOperator(hasOperatorName("*"),
+   has(implicitCastExpr(
+   
hasCastKind(CK_FunctionToPointerDecay
+ .bind("op"),
+ this);
+}
+
+void RedundantFunctionPtrDereferenceCheck::check(const 
MatchFinder::MatchResult &Result) {
+  const auto *Operator = Result.Nodes.getNodeAs("op");
+  diag(Operator->getOperatorLoc(),
+   "redundant repeated dereference of function pointer")
+  << FixItHint::CreateRemoval(Operator->getOperatorLoc());
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

Added: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantFunctionPtrDereferenceCheck.h
URL: 
http://llvm.org/viewvc/llv

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81193.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

last fixes in the code, from alex


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+`
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index:

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I did run clang-tidy with $(./run-clang-tidy.py 
-checks=-*,cppcoreguidelines-no-malloc). The resulting output is in the 
appended file.
Since all warnings came from header files (llvms custom containers?) there are 
always the same warnings for all source files.
Every function (calloc, malloc, realloc and free) was seen, i hope the output 
is sufficient.

Whats not nice is, that there is no underlining (in test neither), but i could 
not find out what is wrong, since i supply a SourceRange in the diag()-call.

F2703383: tidy_output_no_malloc.txt 


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D27673: [clang-move] Only move used helper declarations.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

First round of comments.




Comment at: clang-move/ClangMove.cpp:492
+  isDefinition(), unless(InMovedClass), InOldCC,
+  anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+  auto HelperFuncOrVar = namedDecl(anyOf(functionDecl(IsOldCCHelperDefinition),

It seems that `isStaticStorageClass` is preventing combining matchers for 
functions, variables, and classes. Maybe only apply this matcher on 
`functionDecl` and `varDecl` below, so that helper classes can be matched with 
the same matcher?



Comment at: clang-move/ClangMove.cpp:495
+ varDecl(IsOldCCHelperDefinition)));
+  auto HelperClasses = cxxRecordDecl(
+  InOldCC, isDefinition(), hasDeclContext(namespaceDecl(isAnonymous(;

Why do we need to match classes separately? (Please explain in comments.)



Comment at: clang-move/ClangMove.cpp:514
+  hasParent(namespaceDecl(isAnonymous(;
+  auto DeclMatcher =
+  hasDeclaration(cxxRecordDecl(IsOldCCHelperClass).bind("used_class"));

ClassDeclMatcher?



Comment at: clang-move/ClangMove.cpp:519
+ unless(hasAncestor(namespaceDecl(isAnonymous(,
+ hasAncestor(decl().bind("dc"))),
+ &CGBuilder);

Can we just restrict `"dc"` to be the top level or outermost decls in the first 
place? So that we can guarantee all declarations we are dealing with are those 
that we really care about?  This would simplify the problem a bit I believe; 
otherwise, my gut feeling tells me there would be a lot of things that might go 
wrong.



Comment at: clang-move/ClangMove.cpp:587
+  } else if (const auto *UD = Result.Nodes.getNodeAs(
+ "using_decl_in_anonymous_ns")) {
+MovedDecls.push_back(UD);

What about using declarations in non-anonymous namespaces in old cc? Do we also 
move those?



Comment at: clang-move/ClangMove.cpp:627
+  // If old_header is not specified (only move declarations from old.cc), 
remain
+  // all the helper function declarations in old.cc as UnremovedDecls is empty
+  // in this case.

Why is `UnremovedDecls` empty in this case btw?



Comment at: clang-move/ClangMove.cpp:637
+// declarations.
+UsedHelperDeclFinder::HelperDeclsSet UsedHelperDecls =
+HelperDeclFinder.getUsedHelperDecls(UnremovedDecls);

Maybe a better name for this variable. It's a bit hard to follow which decls 
use these helper decls since there are several sets of decls here like 
moved/removed/unremoved...



Comment at: clang-move/ClangMove.cpp:708
+  UsedHelperDeclFinder HelperDeclFinder(CGBuilder.getCallGraph());
+  llvm::DenseSet HelperDecls =
+  HelperDeclFinder.getUsedHelperDecls(RemovedDecls);

Maybe `RemovedDeclHelpers`?



Comment at: clang-move/ClangMove.cpp:715
+if (!llvm::is_contained(HelperDeclarations, D) ||
+UsedHelperDeclFinder::isUsed(D, HelperDecls))
+  RealNewCCDecls.push_back(D);

IIUC, this condition makes sure helpers used by helpers are moved. If so, 
please explain this in the comment.



Comment at: clang-move/ClangMove.h:93
+// files (old.h/cc) to new files (new.h/cc).
+// The goal of this tool is to make the new/old files as compliable as 
possible.
+// When moving a class, all its/ members are also moved. In addition,

s/compliable/compilable/



Comment at: clang-move/ClangMove.h:95
+// When moving a class, all its/ members are also moved. In addition,
+// all used helper declarations (functions/variables/class definitions in
+// anonymous namespace, static funtioncs/variables), using-declarations in

This also holds for functions. Maybe "when moving a symbol"

"all helper declarations ... used by moved symbols."



Comment at: clang-move/ClangMove.h:100
+//
+// The remaing unused helper declarations in old.cc after moving out the given
+// declarations will also be removed.

Maybe "remaining  helpers that are not used by non-moved symbols"?



Comment at: clang-move/ClangMove.h:166
   std::vector CCIncludes;
+  // Records all helper declarations (functions/variables declared as static or
+  // declared in anonymous namespace) in old.cc, saving in an AST-visited 
order.

Is helper class considered here? 



Comment at: clang-move/UsedHelperDeclFinder.cpp:20
+// outmost eclosing class declaration or function declaration if exists.
+// Because we consider all class method declarations of a class are represented
+// by a single node which belongs to the class.

This sentence doesn't parse?


===

[PATCH] D22507: Clang-tidy - Enum misuse check

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

What is your opinion about the new results? I hope the checker can make it into 
4.0.


https://reviews.llvm.org/D22507



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

r289525 - clang-format: Improve braced-list detection.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Dec 13 04:05:03 2016
New Revision: 289525

URL: http://llvm.org/viewvc/llvm-project?rev=289525&view=rev
Log:
clang-format: Improve braced-list detection.

Before:
  vector v { 12 }
  GUARDED_BY(mutex);

After:
  vector v{12} GUARDED_BY(mutex);

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

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=289525&r1=289524&r2=289525&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Dec 13 04:05:03 2016
@@ -360,8 +360,6 @@ void UnwrappedLineParser::calculateBrace
   // BlockKind later if we parse a braced list (where all blocks
   // inside are by default braced lists), or when we explicitly detect
   // blocks (for example while parsing lambdas).
-  //
-  // We exclude + and - as they can be ObjC visibility modifiers.
   ProbablyBracedList =
   (Style.Language == FormatStyle::LK_JavaScript &&
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
@@ -369,6 +367,8 @@ void UnwrappedLineParser::calculateBrace
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
tok::r_paren, tok::r_square, tok::l_brace,
tok::l_square, tok::l_paren, tok::ellipsis) ||
+  (NextTok->is(tok::identifier) &&
+   !PrevTok->isOneOf(tok::semi, tok::r_brace, tok::l_brace)) ||
   (NextTok->is(tok::semi) &&
(!ExpectClassBody || LBraceStack.size() != 1)) ||
   (NextTok->isBinaryOperator() && !NextIsObjCMethod);

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=289525&r1=289524&r2=289525&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Dec 13 04:05:03 2016
@@ -6504,6 +6504,19 @@ TEST_F(FormatTest, LayoutCxx11BraceIniti
"};");
   verifyFormat("#define A {a, a},");
 
+  // Cases where distinguising braced lists and blocks is hard.
+  verifyFormat("vector v{12} GUARDED_BY(mutex);");
+  verifyFormat("void f() {\n"
+   "  return; // comment\n"
+   "}\n"
+   "SomeType t;");
+  verifyFormat("void f() {\n"
+   "  if (a) {\n"
+   "f();\n"
+   "  }\n"
+   "}\n"
+   "SomeType t;");
+
   // In combination with BinPackArguments = false.
   FormatStyle NoBinPacking = getLLVMStyle();
   NoBinPacking.BinPackArguments = false;


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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:29
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"
+  .bind("aquisition"),

C memory management functions are also present in the `std` namespace.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

Do you mean `#r10-avoid-malloc-and-free`?



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:26
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct 
some_struct));
+

Missing variable.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

malcolm.parsons wrote:
> Do you mean `#r10-avoid-malloc-and-free`?
yes, but i think the wider view is more appropriate. RAII is a general 
strategy, and not specific to malloc etc.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:26
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* = (struct some_struct*) malloc(sizeof(struct 
some_struct));
+

malcolm.parsons wrote:
> Missing variable.
that slipped through. i fix it.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: clang-tidy/cppcoreguidelines/NoMallocCheck.cpp:29
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(hasAnyName("::malloc", "::calloc"
+  .bind("aquisition"),

malcolm.parsons wrote:
> C memory management functions are also present in the `std` namespace.
in the clang codebase, std::malloc got catched as well. see the 
tidy_output_no_malloc.txt in one of my previous comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81204.
JonasToth added a comment.

fix missing variable in doc


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+`
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* s = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index: clang-tidy/cppcoreguidelines/NoMallocCheck.h
==

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

JonasToth wrote:
> malcolm.parsons wrote:
> > Do you mean `#r10-avoid-malloc-and-free`?
> yes, but i think the wider view is more appropriate. RAII is a general 
> strategy, and not specific to malloc etc.
This checker is specific to R.10.
The documentation for all the other cppcoreguidelines checks mention which rule 
they're enforcing.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:4
+cppcoreguidelines-no-malloc
+===
+

A few `=` missing.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 81208.
JonasToth added a comment.

adjusted doc.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
  clang-tidy/cppcoreguidelines/NoMallocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Index: test/clang-tidy/cppcoreguidelines-no-malloc.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-no-malloc.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
+
+using size_t = unsigned long;
+
+void *malloc(size_t size);
+void *calloc(size_t num, size_t size);
+void *realloc(void *ptr, size_t size);
+void free(void *ptr);
+
+void malloced_array() {
+  int *array0 = (int *)malloc(sizeof(int) * 20);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  int *zeroed = (int *)calloc(20, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+
+  // reallocation memory, std::vector shall be used
+  char *realloced = (char *)realloc(array0, 50 * sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not manage memory manually; consider std::vector or std::string [cppcoreguidelines-no-malloc]
+
+  // freeing memory the bad way
+  free(realloced);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+
+  // check if a call to malloc as function argument is found as well
+  free(malloc(20));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]
+}
+
+/// newing an array is still not good, but not relevant to this checker
+void newed_array() {
+  int *new_array = new int[10]; // OK(1)
+}
+
+void arbitrary_call() {
+  // we dont want every function to raise the warning even if malloc is in the name
+  malloced_array(); // OK(2)
+
+  // completly unrelated function call to malloc
+  newed_array(); // OK(3)
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -22,6 +22,7 @@
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-no-malloc
cppcoreguidelines-pro-bounds-array-to-pointer-decay
cppcoreguidelines-pro-bounds-constant-array-index
cppcoreguidelines-pro-bounds-pointer-arithmetic
Index: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - cppcoreguidelines-no-malloc
+
+cppcoreguidelines-no-malloc
+===
+
+This check handles C-Style memory management using ``malloc()``, ``realloc()``, 
+``calloc()`` and ``free()``. It warns about its use and tries to suggest the use 
+of an appropriate RAII object.
+See `C++ Core Guidelines
+
+
+There is no attempt made to provide fixit hints, since manual resource management isn't
+easily transformed automatically into RAII.
+
+.. code-block:: c++
+
+  // Warns each of the following lines.
+  // Containers like std::vector or std::string should be used.
+  char* some_string = (char*) malloc(sizeof(char) * 20); 
+  char* some_string = (char*) realloc(sizeof(char) * 30);
+  free(some_string);
+
+  int* int_array = (int*) calloc(30, sizeof(int)); 
+
+  // Rather use a smartpointer or stack variable.
+  struct some_struct* s = (struct some_struct*) malloc(sizeof(struct some_struct));
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,10 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `cppcoreguidelines-no-malloc 
+  `_ check
+  warns if C-style memory management is used and suggests the use of RAII.
+
 - `modernize-make-unique
   `_
   and `modernize-make-shared
Index: clang-tidy/cppcoreguidelines/NoMallocCheck.h
=

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 4 inline comments as done.
JonasToth added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst:10
+See `C++ Core Guidelines
+`
+

malcolm.parsons wrote:
> JonasToth wrote:
> > malcolm.parsons wrote:
> > > Do you mean `#r10-avoid-malloc-and-free`?
> > yes, but i think the wider view is more appropriate. RAII is a general 
> > strategy, and not specific to malloc etc.
> This checker is specific to R.10.
> The documentation for all the other cppcoreguidelines checks mention which 
> rule they're enforcing.
alright. i will adjust.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18
 using namespace clang::ast_matchers;
+using namespace clang::tidy::utils;
+

I don't think it is required



Comment at: clang-tidy/utils/ExprSequence.cpp:180-182
+}
+}
+}

same here



Comment at: clang-tidy/utils/ExprSequence.h:121-123
+}
+}
+}

Run clang-tidy llvm checks on this patch. These braces requires comments like 
  // namespace clang


https://reviews.llvm.org/D27700



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

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18
 using namespace clang::ast_matchers;
+using namespace clang::tidy::utils;
+

Prazek wrote:
> I don't think it is required
ok I guess I am wrong


https://reviews.llvm.org/D27700



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

r289531 - clang-format: Keep string-literal-label + value pairs on a line.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Dec 13 05:16:42 2016
New Revision: 289531

URL: http://llvm.org/viewvc/llvm-project?rev=289531&view=rev
Log:
clang-format: Keep string-literal-label + value pairs on a line.

We have previously done that for <<-operators. This patch also adds
this logic for "," and "+".

Before:
  string v = ": " +  + ": " +
  + ": " + ;
  string v = StrCat(": ", , ": 
",
, ": ", );

After:
  string v = ": " +  +
 ": " +  +
 ": " + ;
  string v = StrCat(": ", ,
": ", ,
": ", );

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

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=289531&r1=289530&r2=289531&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Tue Dec 13 05:16:42 2016
@@ -396,6 +396,21 @@ struct FormatToken {
 }
   }
 
+  /// \brief Returns \c true if this is a string literal that's like a label,
+  /// e.g. ends with "=" or ":".
+  bool isLabelString() const {
+if (!is(tok::string_literal))
+  return false;
+StringRef Content = TokenText;
+if (Content.startswith("\"") || Content.startswith("'"))
+  Content = Content.drop_front(1);
+if (Content.endswith("\"") || Content.endswith("'"))
+  Content = Content.drop_back(1);
+Content = Content.trim();
+return Content.size() > 1 &&
+   (Content.back() == ':' || Content.back() == '=');
+  }
+
   /// \brief Returns actual token start location without leading escaped
   /// newlines and whitespace.
   ///

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=289531&r1=289530&r2=289531&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Dec 13 05:16:42 2016
@@ -1882,8 +1882,7 @@ unsigned TokenAnnotator::splitPenalty(co
   return 100;
   }
 
-  if (Left.is(tok::comma) || (Right.is(tok::identifier) && Right.Next &&
-  Right.Next->is(TT_DictLiteral)))
+  if (Right.is(tok::identifier) && Right.Next && 
Right.Next->is(TT_DictLiteral))
 return 1;
   if (Right.is(tok::l_square)) {
 if (Style.Language == FormatStyle::LK_Proto)
@@ -1999,21 +1998,17 @@ unsigned TokenAnnotator::splitPenalty(co
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
-  if (Right.is(tok::lessless)) {
-if (Left.is(tok::string_literal) &&
-(Right.NextOperator || Right.OperatorIndex != 1)) {
-  StringRef Content = Left.TokenText;
-  if (Content.startswith("\""))
-Content = Content.drop_front(1);
-  if (Content.endswith("\""))
-Content = Content.drop_back(1);
-  Content = Content.trim();
-  if (Content.size() > 1 &&
-  (Content.back() == ':' || Content.back() == '='))
-return 25;
-}
+  if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
+  Left.Previous->isLabelString() &&
+  (Left.NextOperator || Left.OperatorIndex != 1))
+return 100;
+  if (Left.is(tok::comma))
+return 1;
+  if (Right.isOneOf(tok::lessless, tok::plus) && Left.isLabelString() &&
+  (Right.NextOperator || Right.OperatorIndex != 1))
+return 25;
+  if (Right.is(tok::lessless))
 return 1; // Breaking at a << is really cheap.
-  }
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2631,7 +2626,8 @@ void TokenAnnotator::printDebugInfo(cons
  << " FakeLParens=";
 for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
   llvm::errs() << Tok->FakeLParens[i] << "/";
-llvm::errs() << " FakeRParens=" << Tok->FakeRParens << "\n";
+llvm::errs() << " FakeRParens=" << Tok->FakeRParens;
+llvm::errs() << " Text='" << Tok->TokenText << "'\n";
 if (!Tok->Next)
   assert(Tok == Line.Last);
 Tok = Tok->Next;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=289531&r1=289530&r2=289531&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTe

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:899
+case UO_AddrOf: {
+  // Process pointer-to-member address operation
+  const Expr *Ex = U->getSubExpr()->IgnoreParens();

dcoughlin wrote:
> Just sticking this in the middle of a fallthrough cascade seems quite 
> brittle. For example, it relies on the sub expression of a unary deref never 
> being a DeclRefExpr to a field. This may be guaranteed by Sema (?) but if so 
> it is quite a non-obvious invariant to rely upon.
> 
> I think it would be better the restructure this so that the AddrOf case 
> doesn't fall in the the middle of a fallthrough cascade. You could either 
> factor out the default behavior into a function or use a goto.
It seems that UO_Extension case is the default behavior for this case cascade 
(just below UO_AddrOf). AFAIU it would be better to explicitly call the default 
behavior function following by break for each case preceding UO_Extension 
(UO_Plus,UO_Deref and UO_AddrOf). Or i missed something?


https://reviews.llvm.org/D25475



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

[PATCH] D27199: [libcxx] Make std::ignore constexpr

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D27199#619278, @EricWF wrote:

> @AntonBikineev Do you need somebody to commit this for you?


oh, yeah, it would be great. I don't have commit privileges.


https://reviews.llvm.org/D27199



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

[PATCH] D26830: [libcxx] Add string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D26830#619340, @EricWF wrote:

> Please ping this once the Clang changes have been accepted. I'm just waiting 
> on those to give this the final OK.


I don't have commit privileges to Clang either, so that patch is stuck. Could 
you commit there please?


https://reviews.llvm.org/D26830



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

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Kirill Romanenkov via Phabricator via cfe-commits]

kromanenkov added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:899
+case UO_AddrOf: {
+  // Process pointer-to-member address operation
+  const Expr *Ex = U->getSubExpr()->IgnoreParens();

kromanenkov wrote:
> dcoughlin wrote:
> > Just sticking this in the middle of a fallthrough cascade seems quite 
> > brittle. For example, it relies on the sub expression of a unary deref 
> > never being a DeclRefExpr to a field. This may be guaranteed by Sema (?) 
> > but if so it is quite a non-obvious invariant to rely upon.
> > 
> > I think it would be better the restructure this so that the AddrOf case 
> > doesn't fall in the the middle of a fallthrough cascade. You could either 
> > factor out the default behavior into a function or use a goto.
> It seems that UO_Extension case is the default behavior for this case cascade 
> (just below UO_AddrOf). AFAIU it would be better to explicitly call the 
> default behavior function following by break for each case preceding 
> UO_Extension (UO_Plus,UO_Deref and UO_AddrOf). Or i missed something?
Or maybe just move UO_AddrOf to the beginning of the case cascade and proceed 
with the default behavior if it falls (no need to change UO_Plus and UO_Deref 
in that case)?


https://reviews.llvm.org/D25475



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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Firat Kasmis via Phabricator via cfe-commits]

firolino added a comment.

In https://reviews.llvm.org/D27621#620464, @rsmith wrote:

> Please add a test to make sure this does not fire on C++17 decomposition 
> declarations:
>
>   void f() {
> struct X { int a, b, c; };
> auto [a, b, c] = X();
>   }


Done. Thanks!


https://reviews.llvm.org/D27621



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

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

[Ben Harper via Phabricator via cfe-commits]

bmharper updated this revision to Diff 81213.
bmharper added a comment.

Sorry for the incredibly long turnaround time on this one - I don't have any 
legitimate excuse, and I know it just makes reviewing it harder.

Anyway - I have sorted out all the issues mentioned in the last review. One 
thing that you suggested was to go ahead and replace the std::pair for 
NestingAndIndentLevel, which I did write, but I feel like it adds quite a bit 
of bloat, because you need two constructors, operator==, operator!=, operator<, 
operator>, so I'm not convinced that it's worth it in the end. If you still 
think it is, then I'll go ahead and add it.

Regards,
Ben


https://reviews.llvm.org/D21279

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8612,12 +8612,11 @@
"};",
Alignment);
 
-  // FIXME: Should align all three assignments
   verifyFormat(
   "int i  = 1;\n"
   "SomeType a = SomeFunction(looongParameterA,\n"
   "  loongParameterB);\n"
-  "int j = 2;",
+  "int j  = 2;",
   Alignment);
 
   verifyFormat("template  2) ? 3 : 4);\n"
"float b[1][] = {{3.f}};\n",
Alignment);
+  verifyFormat("for (int i = 0; i < 1; i++)\n"
+   "  int x = 1;\n",
+   Alignment);
+  verifyFormat("for (i = 0; i < 1; i++)\n"
+   "  x = 1;\n"
+   "y = 1;\n",
+   Alignment);
 }
 
 TEST_F(FormatTest, AlignConsecutiveDeclarations) {
@@ -8699,7 +8705,57 @@
"unsigned oneTwoThree = 123;\n"
"int oneTwo = 12;",
Alignment));
+  // Function prototype alignment
+  verifyFormat("inta();\n"
+   "double b();",
+   Alignment);
+  verifyFormat("inta(int x);\n"
+   "double b();",
+   Alignment);
+  unsigned OldColumnLimit = Alignment.ColumnLimit;
+  // We need to set ColumnLimit to zero, in order to stress nested alignments,
+  // otherwise the function parameters will be re-flowed onto a single line.
+  Alignment.ColumnLimit = 0;
+  EXPECT_EQ("inta(int   x,\n"
+" float y);\n"
+"double b(intx,\n"
+" double y);",
+format("int a(int x,\n"
+   " float y);\n"
+   "double b(int x,\n"
+   " double y);",
+   Alignment));
+  // This ensures that function parameters of function declarations are
+  // correctly indented when their owning functions are indented.
+  // The failure case here is for 'double y' to not be indented enough.
+  EXPECT_EQ("double a(int x);\n"
+"intb(inty,\n"
+" double z);",
+format("double a(int x);\n"
+   "int b(int y,\n"
+   " double z);",
+   Alignment));
+  // Set ColumnLimit low so that we induce wrapping immediately after
+  // the function name and opening paren.
+  Alignment.ColumnLimit = 13;
+  verifyFormat("int function(\n"
+   "int  x,\n"
+   "bool y);",
+   Alignment);
+  Alignment.ColumnLimit = OldColumnLimit;
+  // Ensure function pointers don't screw up recursive alignment
+  verifyFormat("inta(int x, void (*fp)(int y));\n"
+   "double b();",
+   Alignment);
   Alignment.AlignConsecutiveAssignments = true;
+  // Ensure recursive alignment is broken by function braces, so that the
+  // "a = 1" does not align with subsequent assignments inside the function
+  // body.
+  verifyFormat("int func(int a = 1) {\n"
+   "  int b  = 2;\n"
+   "  int cc = 3;\n"
+   "}",
+   Alignment);
   verifyFormat("float  something = 2000;\n"
"double another   = 911;\n"
"inti = 1, j = 10;\n"
@@ -8709,6 +8765,28 @@
   verifyFormat("int  oneTwoThree = {0}; // comment\n"
"unsigned oneTwo  = 0;   // comment",
Alignment);
+  // Make sure that scope is correctly tracked, in the absence of braces
+  verifyFormat("for (int i = 0; i < n; i++)\n"
+   "  j = i;\n"
+   "double x = 1;\n",
+   Alignment);
+  verifyFormat("if (int i = 0)\n"
+   "  j = i;\n"
+   "double x = 1;\n",
+   Alignment);
+  // Ensure operator[] and operator() are comprehended
+  verifyFormat("struct test {\n"
+   "  long long int foo();\n"
+   "  int   operator[](int a);\n"
+   "  doublebar();\n"
+   "};\n",
+   Alignment);
+  verifyFormat(

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Firat Kasmis via Phabricator via cfe-commits]

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



Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:154
+  if (const auto *MemberPointerT = Type->getAs()) {
+auto Pos = UserWrittenType.find("::");
+if (Pos != std::string::npos) { // might be hidden behind typedef etc.

malcolm.parsons wrote:
> Can this be confused by comments that contain `::`?
Yes, it does on `int /*next time I will use C::*/ S::*p = ...;`
Working on fix/different getUserWrittenType.


https://reviews.llvm.org/D27621



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

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

[Ilya Palachev via Phabricator via cfe-commits]

ilya-palachev created this revision.
ilya-palachev added reviewers: NoQ, zaks.anna, dcoughlin.
ilya-palachev added subscribers: cfe-commits, a.sidorin, ilya-palachev.
ilya-palachev set the repository for this revision to rL LLVM.

Current implementation doesn't take care about the duplicates in edges
of ExplodedGraph. This actually happens when, for example, the checker
tries to add transition to node, and gets `nullptr', which means that
the node already exists and has been processed. In this case edge
between the node and its predecessor is duplicated.

This patch prohibits this situation, by checking whether the actual
group already contains the given predecessor.


Repository:
  rL LLVM

https://reviews.llvm.org/D27710

Files:
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExplodedGraph.cpp


Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -215,6 +215,11 @@
 
 void ExplodedNode::addPredecessor(ExplodedNode *V, ExplodedGraph &G) {
   assert (!V->isSink());
+  for (ExplodedNode *N : Preds)
+assert(N != V && "Edge already exists");
+  for (ExplodedNode *N : Succs)
+assert(N != this && "Edge already exists");
+
   Preds.addNode(V, G);
   V->Succs.addNode(this, G);
 #ifndef NDEBUG
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -657,7 +657,17 @@
   HasGeneratedNodes = true;
   bool IsNew;
   ExplodedNode *N = C.Eng.G.getNode(Loc, State, MarkAsSink, &IsNew);
-  N->addPredecessor(FromN, C.Eng.G);
+
+  bool EdgeExists = false;
+  for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I)
+if (*I == FromN) {
+  EdgeExists = true;
+  break;
+}
+
+  if (!EdgeExists)
+N->addPredecessor(FromN, C.Eng.G);
+
   Frontier.erase(FromN);
 
   if (!IsNew)


Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -215,6 +215,11 @@
 
 void ExplodedNode::addPredecessor(ExplodedNode *V, ExplodedGraph &G) {
   assert (!V->isSink());
+  for (ExplodedNode *N : Preds)
+assert(N != V && "Edge already exists");
+  for (ExplodedNode *N : Succs)
+assert(N != this && "Edge already exists");
+
   Preds.addNode(V, G);
   V->Succs.addNode(this, G);
 #ifndef NDEBUG
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -657,7 +657,17 @@
   HasGeneratedNodes = true;
   bool IsNew;
   ExplodedNode *N = C.Eng.G.getNode(Loc, State, MarkAsSink, &IsNew);
-  N->addPredecessor(FromN, C.Eng.G);
+
+  bool EdgeExists = false;
+  for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I)
+if (*I == FromN) {
+  EdgeExists = true;
+  break;
+}
+
+  if (!EdgeExists)
+N->addPredecessor(FromN, C.Eng.G);
+
   Frontier.erase(FromN);
 
   if (!IsNew)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

[Ilya Palachev via Phabricator via cfe-commits]

ilya-palachev removed rL LLVM as the repository for this revision.
ilya-palachev updated this revision to Diff 81221.
ilya-palachev added a comment.

Fixed a typo


https://reviews.llvm.org/D27710

Files:
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExplodedGraph.cpp


Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -215,6 +215,11 @@
 
 void ExplodedNode::addPredecessor(ExplodedNode *V, ExplodedGraph &G) {
   assert (!V->isSink());
+  for (ExplodedNode *N : Preds)
+assert(N != V && "Edge already exists");
+  for (ExplodedNode *N : V->Succs)
+assert(N != this && "Edge already exists");
+
   Preds.addNode(V, G);
   V->Succs.addNode(this, G);
 #ifndef NDEBUG
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -657,7 +657,17 @@
   HasGeneratedNodes = true;
   bool IsNew;
   ExplodedNode *N = C.Eng.G.getNode(Loc, State, MarkAsSink, &IsNew);
-  N->addPredecessor(FromN, C.Eng.G);
+
+  bool EdgeExists = false;
+  for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I)
+if (*I == FromN) {
+  EdgeExists = true;
+  break;
+}
+
+  if (!EdgeExists)
+N->addPredecessor(FromN, C.Eng.G);
+
   Frontier.erase(FromN);
 
   if (!IsNew)


Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -215,6 +215,11 @@
 
 void ExplodedNode::addPredecessor(ExplodedNode *V, ExplodedGraph &G) {
   assert (!V->isSink());
+  for (ExplodedNode *N : Preds)
+assert(N != V && "Edge already exists");
+  for (ExplodedNode *N : V->Succs)
+assert(N != this && "Edge already exists");
+
   Preds.addNode(V, G);
   V->Succs.addNode(this, G);
 #ifndef NDEBUG
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -657,7 +657,17 @@
   HasGeneratedNodes = true;
   bool IsNew;
   ExplodedNode *N = C.Eng.G.getNode(Loc, State, MarkAsSink, &IsNew);
-  N->addPredecessor(FromN, C.Eng.G);
+
+  bool EdgeExists = false;
+  for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I)
+if (*I == FromN) {
+  EdgeExists = true;
+  break;
+}
+
+  if (!EdgeExists)
+N->addPredecessor(FromN, C.Eng.G);
+
   Frontier.erase(FromN);
 
   if (!IsNew)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/utils/ExprSequence.cpp:154
+return SyntheticStmtSourceMap.lookup(S);
+  else
+return S;

nit: No `else` after return, please.


https://reviews.llvm.org/D27700



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

[PATCH] D27453: [OpenCL] Enable unroll hint for OpenCL 1.x.

[Egor Churaev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289535: [OpenCL] Enable unroll hint for OpenCL 1.x. 
(authored by echuraev).

Changed prior to commit:
  https://reviews.llvm.org/D27453?vs=80547&id=81223#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27453

Files:
  cfe/trunk/lib/Sema/SemaStmtAttr.cpp
  cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
  cfe/trunk/test/SemaOpenCL/unroll-hint.cl


Index: cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
===
--- cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
+++ cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL1.2 -o - %s | FileCheck %s
 
 /*** for ***/
 void for_count()
Index: cfe/trunk/test/SemaOpenCL/unroll-hint.cl
===
--- cfe/trunk/test/SemaOpenCL/unroll-hint.cl
+++ cfe/trunk/test/SemaOpenCL/unroll-hint.cl
@@ -1,17 +1,5 @@
-//RUN: %clang_cc1 -O0 -fsyntax-only -verify %s
-//RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify -DCL20 %s
+//RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify %s
 
-kernel void D (global int *x) {
-  int i = 10;
-#ifndef CL20
-  // expected-error@+2 {{'opencl_unroll_hint' attribute requires OpenCL 
version 2.0 or above}}
-#endif
-  __attribute__((opencl_unroll_hint))
-  do {
-  } while(i--);
-}
-
-#ifdef CL20
 kernel void C (global int *x) {
   int I = 3;
   __attribute__((opencl_unroll_hint(I))) // expected-error 
{{'opencl_unroll_hint' attribute requires an integer constant}}
@@ -27,4 +15,3 @@
   __attribute__((opencl_unroll_hint(-1))) // expected-error 
{{'opencl_unroll_hint' attribute requires a positive integral compile time 
constant expression}}
   for(int i=0; i<100; i++);
 }
-#endif
Index: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp
@@ -225,16 +225,12 @@
 
 static Attr *handleOpenCLUnrollHint(Sema &S, Stmt *St, const AttributeList &A,
 SourceRange Range) {
-  // OpenCL v2.0 s6.11.5 - opencl_unroll_hint can have 0 arguments (compiler
+  // Although the feature was introduced only in OpenCL C v2.0 s6.11.5, it's
+  // useful for OpenCL 1.x too and doesn't require HW support.
+  // opencl_unroll_hint can have 0 arguments (compiler
   // determines unrolling factor) or 1 argument (the unroll factor provided
   // by the user).
 
-  if (S.getLangOpts().OpenCLVersion < 200) {
-S.Diag(A.getLoc(), diag::err_attribute_requires_opencl_version)
-<< A.getName() << "2.0" << 1;
-return nullptr;
-  }
-
   unsigned NumArgs = A.getNumArgs();
 
   if (NumArgs > 1) {


Index: cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
===
--- cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
+++ cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL1.2 -o - %s | FileCheck %s
 
 /*** for ***/
 void for_count()
Index: cfe/trunk/test/SemaOpenCL/unroll-hint.cl
===
--- cfe/trunk/test/SemaOpenCL/unroll-hint.cl
+++ cfe/trunk/test/SemaOpenCL/unroll-hint.cl
@@ -1,17 +1,5 @@
-//RUN: %clang_cc1 -O0 -fsyntax-only -verify %s
-//RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify -DCL20 %s
+//RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify %s
 
-kernel void D (global int *x) {
-  int i = 10;
-#ifndef CL20
-  // expected-error@+2 {{'opencl_unroll_hint' attribute requires OpenCL version 2.0 or above}}
-#endif
-  __attribute__((opencl_unroll_hint))
-  do {
-  } while(i--);
-}
-
-#ifdef CL20
 kernel void C (global int *x) {
   int I = 3;
   __attribute__((opencl_unroll_hint(I))) // expected-error {{'opencl_unroll_hint' attribute requires an integer constant}}
@@ -27,4 +15,3 @@
   __attribute__((opencl_unroll_hint(-1))) // expected-error {{'opencl_unroll_hint' attribute requires a positive integral compile time constant expression}}
   for(int i=0; i<100; i++);
 }
-#endif
Index: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp
@@ -225,16 +225,12 @@
 
 static Attr *handleOpenCLUnrollHint(Sema &S, Stmt *St, const AttributeList &A,
 SourceRange Range) {
-  // OpenCL v2.0 s6.11.5 - opencl_unroll_hint can have 0 arguments (compiler
+  // Although the feature was introduced only in OpenCL C v2.0 s6.11.5, it's
+  // useful for OpenCL 1.x too and doesn't require HW support.
+  // opencl_unroll_hint can have 0 arguments (compiler
   // determines unrolling factor) or 1 argument 

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG, but please wait until Malcolm

> Whats not nice is, that there is no underlining (in test neither), but i 
> could not find out what is wrong,
>  since i supply a SourceRange in the diag()-call.

Please leave the range anyway. I suspect clang-tidy doesn't handle ranges in 
diagnostics correctly, but it's easier to fix this, if there is an example 
already.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Hit "submit" too early.

LG, but please wait until Malcolm is happy with the change as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

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

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

Now isInStdNamespace is used. Hack is back so https://reviews.llvm.org/D27202 
is not a dependency now.


https://reviews.llvm.org/D25660

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/inlining/stl.cpp
  test/Analysis/iterator-past-end.cpp

Index: test/Analysis/iterator-past-end.cpp
===
--- /dev/null
+++ test/Analysis/iterator-past-end.cpp
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorPastEnd -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorPastEnd -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void simple_good(const std::vector &v) {
+  auto i = v.end();
+  if (i != v.end())
+*i; // no-warning
+}
+
+void simple_good_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end()))
+*i; // no-warning
+}
+
+void simple_bad(const std::vector &v) {
+  auto i = v.end();
+  *i; // expected-warning{{Iterator accessed past its end}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find(std::vector &vec, int e) {
+  auto first = std::find(vec.begin(), vec.end(), e);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find(std::vector &vec, int e) {
+  auto first = std::find(vec.begin(), vec.end(), e);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_end(std::vector &vec, std::vector &seq) {
+  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  if (vec.end() != last)
+*last; // no-warning
+}
+
+void bad_find_end(std::vector &vec, std::vector &seq) {
+  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  *last; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_first_of(std::vector &vec, std::vector &seq) {
+  auto first =
+  std::find_first_of(vec.begin(), vec.end(), seq.begin(), seq.end());
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_first_of(std::vector &vec, std::vector &seq) {
+  auto first = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+bool odd(int i) { return i % 2; }
+
+void good_find_if(std::vector &vec) {
+  auto first = std::find_if(vec.begin(), vec.end(), odd);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_if(std::vector &vec, int e) {
+  auto first = std::find_if(vec.begin(), vec.end(), odd);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_find_if_not(std::vector &vec) {
+  auto first = std::find_if_not(vec.begin(), vec.end(), odd);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_find_if_not(std::vector &vec, int e) {
+  auto first = std::find_if_not(vec.begin(), vec.end(), odd);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_lower_bound(std::vector &vec, int e) {
+  auto first = std::lower_bound(vec.begin(), vec.end(), e);
+  if (vec.end() != first)
+*first; // no-warning
+}
+
+void bad_lower_bound(std::vector &vec, int e) {
+  auto first = std::lower_bound(vec.begin(), vec.end(), e);
+  *first; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_upper_bound(std::vector &vec, int e) {
+  auto last = std::lower_bound(vec.begin(), vec.end(), e);
+  if (vec.end() != last)
+*last; // no-warning
+}
+
+void bad_upper_bound(std::vector &vec, int e) {
+  auto last = std::lower_bound(vec.begin(), vec.end(), e);
+  *last; // expected-warning{{Iterator accessed past its

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 81227.
sepavloff added a comment.

Addressed review notes.


https://reviews.llvm.org/D24933

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Config/config.h.cmake
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-1.cfg
  test/Driver/Inputs/config-2.cfg
  test/Driver/Inputs/config-2a.cfg
  test/Driver/Inputs/config-3.cfg
  test/Driver/Inputs/config-4.cfg
  test/Driver/Inputs/config-5.cfg
  test/Driver/Inputs/config/config-4.cfg
  test/Driver/config-file.c
  test/Driver/config-file2.c
  test/Driver/lit.local.cfg
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -305,6 +305,41 @@
   return 1;
 }
 
+// Directories searched for configuration specified by option '--config'.
+static const ArrayRef SearchDirs = {
+#if defined(CLANG_CONFIG_FILE_USER_DIR)
+  CLANG_CONFIG_FILE_USER_DIR,
+#endif
+#if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
+  CLANG_CONFIG_FILE_SYSTEM_DIR
+#endif
+};
+
+/// Deduce configuration name if it is encoded in the executable name.
+///
+/// \param ConfigFile [out] Is assigned configuration file path.
+/// \param ProgramName [in] clang executable path.
+/// \return True if configuration file was found.
+///
+/// If clang executable is named e.g. 'armv7l-clang' the function tries to
+/// find config file 'armv7l.cfg'. If it is found, its path is put into
+/// ConfigFile and the function returns true.
+///
+static bool findConfigFileFromProgramName(
+llvm::SmallVectorImpl &ConfigFile, StringRef ProgramName) {
+  ConfigFile.clear();
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos == StringRef::npos)
+return false;
+
+  ConfigFile.append(PName.begin(), PName.begin() + Pos);
+  const StringRef Ext(".cfg");
+  ConfigFile.append(Ext.begin(), Ext.end());
+  std::string CName(ConfigFile.begin(), ConfigFile.size());
+  return llvm::cl::searchForFile(ConfigFile, SearchDirs, ProgramName, CName);
+}
+
 int main(int argc_, const char **argv_) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv_[0]);
   llvm::PrettyStackTraceProgram X(argc_, argv_);
@@ -330,6 +365,31 @@
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
+  // Try reading options from configuration file.
+  llvm::SmallString<128> ConfigFile;
+  llvm::cl::SearchResult SRes;
+
+  // First try config file specified in command line. It has higher priority
+  // than any other way to specify configuration.
+  SRes = llvm::cl::findConfigFileFromArgs(ConfigFile, argv, SearchDirs, true);
+  if (llvm::cl::checkConfigFileSearchResult(SRes, ConfigFile, SearchDirs, true,
+ProgName))
+return 1;
+
+  // If config file is not specified explicitly, try to determine configuration
+  // implicitly. First try to deduce configuration from executable name. For
+  // instance, a file 'armv7l-clang' applies config file 'armv7l.cfg'. Second,
+  // try to find file 'clang.cfg'.
+  if (SRes == llvm::cl::SearchResult::NotSpecified) {
+if (findConfigFileFromProgramName(ConfigFile, ProgName))
+  SRes = llvm::cl::SearchResult::Successful;
+  }
+
+  // If config file is found, read options from it.
+  unsigned NumConfigOptions = 0;
+  if (SRes == llvm::cl::SearchResult::Successful)
+llvm::cl::readConfigFile(ConfigFile, Saver, argv, NumConfigOptions);
+
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
@@ -400,8 +460,12 @@
   SmallVector PrependedOpts;
   getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
-  // Insert right after the program name to prepend to the argument list.
-  argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+  // Insert right after the program name to prepend to the argument list. If
+  // there are options read from config file, put the options from "CL"
+  // after them, - config file is considered as a "patch" to compiler
+  // defaults.
+  argv.insert(argv.begin() + 1 + NumConfigOptions,
+  PrependedOpts.begin(), PrependedOpts.end());
 }
 // Arguments in "_CL_" are appended.
 llvm::Optional Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
@@ -446,6 +510,8 @@
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
+  if (!ConfigFile.empty())
+TheDriver.setConfigFile(ConfigFile.str(), NumConfigOptions);
   SetInstallDir(argv, TheDriver, CanonicalPrefixes);
 
   insertTargetAndModeArgs(TargetAndMode.first, TargetAndMode.second, argv,
Index: test/Driver/lit.local.cfg
===

[PATCH] D27284: [ClangTidy] Add new performance-type-promotion-in-math-fn check.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG with one nit.




Comment at: 
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp:145-153
+  bool StdFnRequresCpp11 =
+  llvm::StringSwitch(OldFnName)
+  .Cases("acosh", "asinh", "atanh", "cbrt", "copysign", "erf", "erfc",
+ "exp2", "expm1", "fdim", true)
+  .Cases("fma", "fmax", "fmin", "hypot", "ilogb", "lgamma", "llrint",
+ "llround", "log1p", true)
+  .Cases("log2", "logb", "lrint", "lround", "nearbyint", "nextafter",

Maybe just make a static StringSet of all the names and check whether the name 
is in it?



Comment at: 
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp:167-168
+
+  // FIXME: Perhaps we should suggest #include  if we suggest a cmath
+  // function and cmath is not already included.
+}

We definitely should. See the use of IncludeInserter in 
UnnecessaryValueParamCheck, for example. Fine for a follow-up.


https://reviews.llvm.org/D27284



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

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/utils/ExprSequence.cpp:154
+return SyntheticStmtSourceMap.lookup(S);
+  else
+return S;

alexfh wrote:
> nit: No `else` after return, please.
Not sure if he should change it in this patch - it is just move of this class 
to different file, so I am not sure if it is good do introduce small changes to 
it now.
I guess pushin NFC patch with this after would be good solution


https://reviews.llvm.org/D27700



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

[PATCH] D22507: Clang-tidy - Enum misuse check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

LG with one nit. Feel free to ping earlier next time.




Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:170-171
+  if (const auto *EnumExpr = Result.Nodes.getNodeAs("enumExpr")) {
+if (!StrictMode)
+  return;
+const auto *EnumDec = Result.Nodes.getNodeAs("enumDecl");

Looks like this is the same as in case 3 below, so you could just move this 
check out of the branch and remove the duplication below.


https://reviews.llvm.org/D22507



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons accepted this revision.
malcolm.parsons added a comment.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: include/clang/Driver/Driver.h:287
+  const std::string &getConfigFile() const { return ConfigFile; }
+  void setConfigFile(StringRef x, unsigned N) {
+ConfigFile = x;

bruno wrote:
> x -> FileName
Fixed.



Comment at: lib/Driver/Driver.cpp:172
+NumConfigArgs = static_cast(NumCfgArgs);
+  }
+

bruno wrote:
> If `NumCfgArgs == 0` it would be nice to warn that the config file is empty?
Not sure if it makes sense. Usually warning are used to attract attention to 
the things that potentially may be harmful. An empty config file is strange but 
does not look dangerous. 



Comment at: lib/Driver/Driver.cpp:2698
+  for (unsigned I = 0; I < NumConfigArgs; ++I)
+C.getArgs().getArgs()[I]->claim();
+

bruno wrote:
> Why shouldn't we warn about those? Should clang warn and point to the config 
> file instead?
If clang is called to compile files only, it will warn on options like 
`-L/usr/local/lib` as they are unused. We don't want such warnings for options 
in config file, as it is used for all invocations of clang.



Comment at: tools/driver/driver.cpp:318
+
+/// Deduce configuration name if it is encoded in the executable name.
+///

bruno wrote:
> Use  `\brief` here?
Doxygen is run with AUTO_BRIEF options turned on (enabled in r242485 - Doxygen: 
Enable autobrief feature, matching llvm config/coding standards).



Comment at: tools/driver/driver.cpp:333
+  size_t Pos = PName.find("-clang");
+  if (Pos != StringRef::npos) {
+ConfigFile.append(PName.begin(), PName.begin() + Pos);

bruno wrote:
> You can early `return false` here if `Pos == StringRef::npos`
Fixed.


https://reviews.llvm.org/D24933



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

[PATCH] D22507: Clang-tidy - Enum misuse check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/misc/SuspiciousEnumUsageCheck.cpp:155
+
+if (EnumDec->enumerator_begin() == EnumDec->enumerator_end() ||
+OtherEnumDec->enumerator_begin() == OtherEnumDec->enumerator_end())

szepet wrote:
> alexfh wrote:
> > Why?
> Because the hasDisjointValueRange function could not decide the values 
> properly. So in case of an empty Enum it would not make sense. Fortunately we 
> know that the empty case should not be reported so used early return on this.
> 
> That is why this is needed if we want a deterministic check. 
BTW, this might make sense to be explained in the comment in the code itself 
(code review comments are bad means of documenting code).


https://reviews.llvm.org/D22507



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

[PATCH] D21298: [Clang-tidy] delete null check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:52
+
+  auto D = diag(
+  IfWithDelete->getLocStart(),

Rename `D` to `Diag`, please.



Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:55
+  "'if' statement is unnecessary; deleting null pointer has no effect");
+  if (!IfWithDelete->getElse()) {
+std::string ReplacementText = Lexer::getSourceText(

hokein wrote:
> I would use an early return `if (IfWithDelete->getElse()) return` here.
Definitely use early exit.



Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:56
+  if (!IfWithDelete->getElse()) {
+std::string ReplacementText = Lexer::getSourceText(
+CharSourceRange::getTokenRange(

This variable is unused.



Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:72-76
+  D << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+  Compound->getRBracLoc(),
+  Lexer::getLocForEndOfToken(Compound->getRBracLoc(), 0,
+ *Result.SourceManager,
+ Result.Context->getLangOpts(;

Please clang-format the file.



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:20
+  if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]
+// another comment to keep

Please truncate repeated static parts of the check patterns that exceed 80 
characters (e.g. remove the `deleting null pointer has no effect 
[readability-delete-null-pointer]` part from all but the first CHECK line).



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:53
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]
+delete c2;
+  } else {

Please add CHECK-FIXES lines. Now there's no easy way to see from the test 
whether any fixes are applied here.


https://reviews.llvm.org/D21298



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

[PATCH] D27713: [clang-move] Fix incorrect EndLoc for declarations in macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.

https://reviews.llvm.org/D27713

Files:
  clang-move/ClangMove.cpp
  unittests/clang-move/ClangMoveTests.cpp


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -386,6 +386,24 @@
   EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
 }
 
+TEST(ClangMove, DefinitionInMacro) {
+  const char TestHeader[] = "#define DEF(CLASS) void CLASS##_::f() {}\n"
+"class A_ {\nvoid f();\n};\n"
+"class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n"
+  "DEF(A)\n";
+  const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
+ "DEF(A)\n";
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("A_");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, WellFormattedCode) {
   const std::string CommonHeader =
   "namespace a {\n"
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -219,7 +219,8 @@
 getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
   const auto &SM = D->getASTContext().getSourceManager();
-  std::pair LocInfo = SM.getDecomposedLoc(D->getLocEnd());
+  auto EndExpansionLoc = SM.getExpansionLoc(D->getLocEnd());
+  std::pair LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
   // Try to load the file buffer.
   bool InvalidTemp = false;
   llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
@@ -235,7 +236,7 @@
   // FIXME: this is a bit hacky to get ReadToEndOfLine work.
   Lex.setParsingPreprocessorDirective(true);
   Lex.ReadToEndOfLine(&Line);
-  SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+  SourceLocation EndLoc = EndExpansionLoc.getLocWithOffset(Line.size());
   // If we already reach EOF, just return the EOF SourceLocation;
   // otherwise, move 1 offset ahead to include the trailing newline character
   // '\n'.


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -386,6 +386,24 @@
   EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
 }
 
+TEST(ClangMove, DefinitionInMacro) {
+  const char TestHeader[] = "#define DEF(CLASS) void CLASS##_::f() {}\n"
+"class A_ {\nvoid f();\n};\n"
+"class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n"
+  "DEF(A)\n";
+  const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
+ "DEF(A)\n";
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("A_");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, WellFormattedCode) {
   const std::string CommonHeader =
   "namespace a {\n"
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -219,7 +219,8 @@
 getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
   const auto &SM = D->getASTContext().getSourceManager();
-  std::pair LocInfo = SM.getDecomposedLoc(D->getLocEnd());
+  auto EndExpansionLoc = SM.getExpansionLoc(D->getLocEnd());
+  std::pair LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
   // Try to load the file buffer.
   bool InvalidTemp = false;
   llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
@@ -235,7 +236,7 @@
   // FIXME: this is a bit hacky to get ReadToEndOfLine work.
   Lex.setParsingPreprocessorDirective(true);
   Lex.ReadToEndOfLine(&Line);
-  SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+  SourceLocation EndLoc = EndExpansionLoc.getLocWithOffset(Line.size());
   // If we already reach EOF, just return the EOF SourceLocation;
   // otherwise, move 1 offset ahead to include the trailing newline character
   // '\n'.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27713: [clang-move] Fix incorrect EndLoc for declarations in macros.

[Eric Liu via Phabricator via cfe-commits]

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

Lg


https://reviews.llvm.org/D27713



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

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/misc/StringCompareCheck.cpp:48
+diag(Matched->getLocStart(),
+ "do not use compare to test equality of strings; "
+ "use the string equality operator instead");

`compare` should be enclosed in single quotes (`'compare'`), since it refers to 
a method.



Comment at: docs/clang-tidy/checks/misc-string-compare.rst:12
+zero depending on the lexicographical relationship between the strings 
compared. 
+If an equality or inequality check can suffice, that is recommended.
+

The documentation doesn't explain why is using `compare` for equality 
comparison should be avoided. Maybe `that is recommended to avoid the risk of 
incorrect interpretation of the return value and simplify the code`?


https://reviews.llvm.org/D27210



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

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/utils/ExprSequence.cpp:154
+return SyntheticStmtSourceMap.lookup(S);
+  else
+return S;

Prazek wrote:
> alexfh wrote:
> > nit: No `else` after return, please.
> Not sure if he should change it in this patch - it is just move of this class 
> to different file, so I am not sure if it is good do introduce small changes 
> to it now.
> I guess pushin NFC patch with this after would be good solution
For me it's usually easier to fix than to postpone to a different patch. Feel 
free to do either.


https://reviews.llvm.org/D27700



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

[PATCH] D27284: [ClangTidy] Add new performance-type-promotion-in-math-fn check.

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D27284#609937, @Eugene.Zelenko wrote:

> Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).


This has not been done.


https://reviews.llvm.org/D27284



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

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Jakub Staroń via Phabricator via cfe-commits]

staronj added inline comments.



Comment at: clang-tidy/utils/ExprSequence.cpp:52
+
+bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
+ ASTContext *Context) {

Shouldn't isDescendantOrEqual be static or in inline namespace?


https://reviews.llvm.org/D27700



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

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

[Renato Golin via Phabricator via cfe-commits]

rengolin resigned from this revision.
rengolin removed a reviewer: rengolin.
rengolin added a comment.

I don't know enough about the x86 environment to be able to review this patch. 
Sorry.


https://reviews.llvm.org/D26796



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

[clang-tools-extra] r289541 - [clang-move] Fix incorrect EndLoc for declarations in macros.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Dec 13 09:35:47 2016
New Revision: 289541

URL: http://llvm.org/viewvc/llvm-project?rev=289541&view=rev
Log:
[clang-move] Fix incorrect EndLoc for declarations in macros.

Reviewers: ioeric

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp

Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=289541&r1=289540&r2=289541&view=diff
==
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Tue Dec 13 09:35:47 2016
@@ -219,7 +219,8 @@ SourceLocation
 getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
   const auto &SM = D->getASTContext().getSourceManager();
-  std::pair LocInfo = SM.getDecomposedLoc(D->getLocEnd());
+  auto EndExpansionLoc = SM.getExpansionLoc(D->getLocEnd());
+  std::pair LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
   // Try to load the file buffer.
   bool InvalidTemp = false;
   llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
@@ -235,7 +236,7 @@ getLocForEndOfDecl(const clang::Decl *D,
   // FIXME: this is a bit hacky to get ReadToEndOfLine work.
   Lex.setParsingPreprocessorDirective(true);
   Lex.ReadToEndOfLine(&Line);
-  SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+  SourceLocation EndLoc = EndExpansionLoc.getLocWithOffset(Line.size());
   // If we already reach EOF, just return the EOF SourceLocation;
   // otherwise, move 1 offset ahead to include the trailing newline character
   // '\n'.

Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=289541&r1=289540&r2=289541&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Tue Dec 13 
09:35:47 2016
@@ -386,6 +386,24 @@ TEST(ClangMove, MacroInFunction) {
   EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
 }
 
+TEST(ClangMove, DefinitionInMacro) {
+  const char TestHeader[] = "#define DEF(CLASS) void CLASS##_::f() {}\n"
+"class A_ {\nvoid f();\n};\n"
+"class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n"
+  "DEF(A)\n";
+  const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
+ "DEF(A)\n";
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("A_");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, WellFormattedCode) {
   const std::string CommonHeader =
   "namespace a {\n"


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

[PATCH] D27713: [clang-move] Fix incorrect EndLoc for declarations in macros.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289541: [clang-move] Fix incorrect EndLoc for declarations 
in macros. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D27713?vs=81231&id=81234#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27713

Files:
  clang-tools-extra/trunk/clang-move/ClangMove.cpp
  clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp


Index: clang-tools-extra/trunk/clang-move/ClangMove.cpp
===
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp
@@ -219,7 +219,8 @@
 getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
   const auto &SM = D->getASTContext().getSourceManager();
-  std::pair LocInfo = SM.getDecomposedLoc(D->getLocEnd());
+  auto EndExpansionLoc = SM.getExpansionLoc(D->getLocEnd());
+  std::pair LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
   // Try to load the file buffer.
   bool InvalidTemp = false;
   llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
@@ -235,7 +236,7 @@
   // FIXME: this is a bit hacky to get ReadToEndOfLine work.
   Lex.setParsingPreprocessorDirective(true);
   Lex.ReadToEndOfLine(&Line);
-  SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+  SourceLocation EndLoc = EndExpansionLoc.getLocWithOffset(Line.size());
   // If we already reach EOF, just return the EOF SourceLocation;
   // otherwise, move 1 offset ahead to include the trailing newline character
   // '\n'.
Index: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
@@ -386,6 +386,24 @@
   EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
 }
 
+TEST(ClangMove, DefinitionInMacro) {
+  const char TestHeader[] = "#define DEF(CLASS) void CLASS##_::f() {}\n"
+"class A_ {\nvoid f();\n};\n"
+"class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n"
+  "DEF(A)\n";
+  const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
+ "DEF(A)\n";
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("A_");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, WellFormattedCode) {
   const std::string CommonHeader =
   "namespace a {\n"


Index: clang-tools-extra/trunk/clang-move/ClangMove.cpp
===
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp
@@ -219,7 +219,8 @@
 getLocForEndOfDecl(const clang::Decl *D,
const LangOptions &LangOpts = clang::LangOptions()) {
   const auto &SM = D->getASTContext().getSourceManager();
-  std::pair LocInfo = SM.getDecomposedLoc(D->getLocEnd());
+  auto EndExpansionLoc = SM.getExpansionLoc(D->getLocEnd());
+  std::pair LocInfo = SM.getDecomposedLoc(EndExpansionLoc);
   // Try to load the file buffer.
   bool InvalidTemp = false;
   llvm::StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
@@ -235,7 +236,7 @@
   // FIXME: this is a bit hacky to get ReadToEndOfLine work.
   Lex.setParsingPreprocessorDirective(true);
   Lex.ReadToEndOfLine(&Line);
-  SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+  SourceLocation EndLoc = EndExpansionLoc.getLocWithOffset(Line.size());
   // If we already reach EOF, just return the EOF SourceLocation;
   // otherwise, move 1 offset ahead to include the trailing newline character
   // '\n'.
Index: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
===
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
@@ -386,6 +386,24 @@
   EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
 }
 
+TEST(ClangMove, DefinitionInMacro) {
+  const char TestHeader[] = "#define DEF(CLASS) void CLASS##_::f() {}\n"
+"class A_ {\nvoid f();\n};\n"
+"class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n"
+  "DEF(A)\n";
+  const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
+ "DEF(A)\n";
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("A_");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  a

[PATCH] D27700: [clang-tidy] refactor ExprSequence out of misc-use-after-move check

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/utils/ExprSequence.cpp:52
+
+bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
+ ASTContext *Context) {

staronj wrote:
> Shouldn't isDescendantOrEqual be static or in inline namespace?
Goot catch. I guess putting it with getParentStmts into anonymous namespace is 
the best solution.
btw inline namespace is not the same as anonymous namespace :)


https://reviews.llvm.org/D27700



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

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

In https://reviews.llvm.org/D27166#617696, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D27166#617621, @Prazek wrote:.
>
> > Does it work for cases like?
>
>
> Yes; `replaceExpr()` checks that the variable has the same unqualified type 
> as the initializer and the same canonical type as other variables in the 
> declaration.


Can you add this small test?




Comment at: clang-tidy/modernize/UseAutoCheck.cpp:173-177
+/// Matches the type that was substituted for the template parameter.
+AST_MATCHER_P(SubstTemplateTypeParmType, hasReplacementType,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  return InnerMatcher.matches(Node.getReplacementType(), Finder, Builder);
+}

alexfh wrote:
> Ideally, this should go to ASTMatchers.h (with a proper test and 
> documentation).
I agree 



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:256-264
+  return declStmt(
+ 
unless(has(varDecl(unless(hasInitializer(ignoringImplicit(callExpr(
+ anyOf(has(memberExpr(hasExplicitTemplateArgs())),
+   has(ignoringImpCasts(
+   declRefExpr(hasExplicitTemplateArgs(),
+ callee(functionDecl(
+ hasTemplateArgument(0,

Can you split this matcher into 3 matchers? It is so large that even 
clang-format doesn't help with it.
And also because of that I can't come up with good auto variables to use here, 
because I don't know where the parens ends.



Comment at: docs/clang-tidy/checks/modernize-use-auto.rst:165
+that behave as casts, such as ``llvm::dyn_cast``, ``boost::lexical_cast`` and
+``gsl::narrow_cast``.
 

I would add to that what functions are considered (functions returning type 
chosen as first template parameter or something similar). Now someone could 
think that there is hardcoded list somewhere.


https://reviews.llvm.org/D27166



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

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:21
+
+static StringRef getValueInit(const CXXCtorInitializer *Init) {
+  switch (Init->getInit()->getType()->getScalarTypeKind()) {

The function name doesn't make it clear that the value is returned for 
zero-initialization.



Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:137
+  isDefaultConstructor(),
+  unless(ast_matchers::isTemplateInstantiation()),
+  forEachConstructorInitializer(allOf(

`isInTemplateInstantiation()` is usually a better choice, since it also checks 
for ancestors being templates.



Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:157-161
+  const auto *Default = Result.Nodes.getNodeAs("default");
+  const auto *Existing = 
Result.Nodes.getNodeAs("existing");
+
+  if (Default)
+checkDefaultInit(Result, Default);

This is a more common way to do this in LLVM:
```
if (const auto *Default = Result.Nodes.getNodeAs<...>("default"))
  checkDefaultInit(Result, Default);
else if (const auto *Existing = ...)
  checkExistingInit(...);
else
  llvm_unreachable(...);
```



Comment at: docs/clang-tidy/checks/modernize-use-default-member-init.rst:29
+
+.. note::
+  Only converts member initializers for built-in types, enums and pointers.

Is an empty line needed after this?


https://reviews.llvm.org/D26750



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

[clang-tools-extra] r289542 - Remove deprecated methods ast_matchers::BoundNodes::{getStmtAs, getDeclAs}

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Dec 13 10:19:19 2016
New Revision: 289542

URL: http://llvm.org/viewvc/llvm-project?rev=289542&view=rev
Log:
Remove deprecated methods ast_matchers::BoundNodes::{getStmtAs,getDeclAs}

Modified:

clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/IncorrectRoundings.cpp
clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp?rev=289542&r1=289541&r2=289542&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp 
Tue Dec 13 10:19:19 2016
@@ -32,8 +32,8 @@ void BoolPointerImplicitConversionCheck:
 
 void BoolPointerImplicitConversionCheck::check(
 const MatchFinder::MatchResult &Result) {
-  auto *If = Result.Nodes.getStmtAs("if");
-  auto *Var = Result.Nodes.getStmtAs("expr");
+  auto *If = Result.Nodes.getNodeAs("if");
+  auto *Var = Result.Nodes.getNodeAs("expr");
 
   // Ignore macros.
   if (Var->getLocStart().isMacroID())

Modified: clang-tools-extra/trunk/clang-tidy/misc/IncorrectRoundings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/IncorrectRoundings.cpp?rev=289542&r1=289541&r2=289542&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/IncorrectRoundings.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/IncorrectRoundings.cpp Tue Dec 13 
10:19:19 2016
@@ -60,7 +60,7 @@ void IncorrectRoundings::registerMatcher
 }
 
 void IncorrectRoundings::check(const MatchFinder::MatchResult &Result) {
-  const auto *CastExpr = Result.Nodes.getStmtAs("CastExpr");
+  const auto *CastExpr = Result.Nodes.getNodeAs("CastExpr");
   diag(CastExpr->getLocStart(),
"casting (double + 0.5) to integer leads to incorrect rounding; "
"consider using lround (#include ) instead");

Modified: clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp?rev=289542&r1=289541&r2=289542&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/SwappedArgumentsCheck.cpp Tue Dec 
13 10:19:19 2016
@@ -49,7 +49,7 @@ static bool isImplicitCastCandidate(cons
 
 void SwappedArgumentsCheck::check(const MatchFinder::MatchResult &Result) {
   const ASTContext &Ctx = *Result.Context;
-  const auto *Call = Result.Nodes.getStmtAs("call");
+  const auto *Call = Result.Nodes.getNodeAs("call");
 
   llvm::SmallPtrSet UsedArgs;
   for (unsigned I = 1, E = Call->getNumArgs(); I < E; ++I) {

Modified: clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp?rev=289542&r1=289541&r2=289542&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UndelegatedConstructor.cpp Tue Dec 
13 10:19:19 2016
@@ -74,7 +74,7 @@ void UndelegatedConstructorCheck::regist
 
 void UndelegatedConstructorCheck::check(
 const MatchFinder::MatchResult &Result) {
-  const auto *E = Result.Nodes.getStmtAs("construct");
+  const auto *E = Result.Nodes.getNodeAs("construct");
   diag(E->getLocStart(), "did you intend to call a delegated constructor? "
  "A temporary object is created here instead");
 }

Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp?rev=289542&r1=289541&r2=289542&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp Tue Dec 13 
10:19:19 2016
@@ -48,7 +48,7 @@ void UnusedRAIICheck::registerMatchers(M
 }
 
 void Un

r289543 - Remove deprecated methods ast_matchers::BoundNodes::{getStmtAs, getDeclAs}

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Dec 13 10:19:34 2016
New Revision: 289543

URL: http://llvm.org/viewvc/llvm-project?rev=289543&view=rev
Log:
Remove deprecated methods ast_matchers::BoundNodes::{getStmtAs,getDeclAs}

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/Tooling/RefactoringCallbacks.cpp
cfe/trunk/unittests/AST/DeclPrinterTest.cpp
cfe/trunk/unittests/AST/StmtPrinterTest.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=289543&r1=289542&r2=289543&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Tue Dec 13 10:19:34 2016
@@ -75,18 +75,6 @@ public:
 return MyBoundNodes.getNodeAs(ID);
   }
 
-  /// \brief Deprecated. Please use \c getNodeAs instead.
-  /// @{
-  template 
-  const T *getDeclAs(StringRef ID) const {
-return getNodeAs(ID);
-  }
-  template 
-  const T *getStmtAs(StringRef ID) const {
-return getNodeAs(ID);
-  }
-  /// @}
-
   /// \brief Type of mapping from binding identifiers to bound nodes. This type
   /// is an associative container with a key type of \c std::string and a value
   /// type of \c clang::ast_type_traits::DynTypedNode

Modified: cfe/trunk/lib/Tooling/RefactoringCallbacks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/RefactoringCallbacks.cpp?rev=289543&r1=289542&r2=289543&view=diff
==
--- cfe/trunk/lib/Tooling/RefactoringCallbacks.cpp (original)
+++ cfe/trunk/lib/Tooling/RefactoringCallbacks.cpp Tue Dec 13 10:19:34 2016
@@ -39,7 +39,7 @@ ReplaceStmtWithText::ReplaceStmtWithText
 
 void ReplaceStmtWithText::run(
 const ast_matchers::MatchFinder::MatchResult &Result) {
-  if (const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId)) {
+  if (const Stmt *FromMatch = Result.Nodes.getNodeAs(FromId)) {
 auto Err = Replace.add(tooling::Replacement(
 *Result.SourceManager,
 CharSourceRange::getTokenRange(FromMatch->getSourceRange()), ToText));
@@ -56,8 +56,8 @@ ReplaceStmtWithStmt::ReplaceStmtWithStmt
 
 void ReplaceStmtWithStmt::run(
 const ast_matchers::MatchFinder::MatchResult &Result) {
-  const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId);
-  const Stmt *ToMatch = Result.Nodes.getStmtAs(ToId);
+  const Stmt *FromMatch = Result.Nodes.getNodeAs(FromId);
+  const Stmt *ToMatch = Result.Nodes.getNodeAs(ToId);
   if (FromMatch && ToMatch) {
 auto Err = Replace.add(
 replaceStmtWithStmt(*Result.SourceManager, *FromMatch, *ToMatch));
@@ -75,7 +75,7 @@ ReplaceIfStmtWithItsBody::ReplaceIfStmtW
 
 void ReplaceIfStmtWithItsBody::run(
 const ast_matchers::MatchFinder::MatchResult &Result) {
-  if (const IfStmt *Node = Result.Nodes.getStmtAs(Id)) {
+  if (const IfStmt *Node = Result.Nodes.getNodeAs(Id)) {
 const Stmt *Body = PickTrueBranch ? Node->getThen() : Node->getElse();
 if (Body) {
   auto Err =

Modified: cfe/trunk/unittests/AST/DeclPrinterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/DeclPrinterTest.cpp?rev=289543&r1=289542&r2=289543&view=diff
==
--- cfe/trunk/unittests/AST/DeclPrinterTest.cpp (original)
+++ cfe/trunk/unittests/AST/DeclPrinterTest.cpp Tue Dec 13 10:19:34 2016
@@ -45,7 +45,7 @@ public:
   PrintMatch() : NumFoundDecls(0) {}
 
   void run(const MatchFinder::MatchResult &Result) override {
-const Decl *D = Result.Nodes.getDeclAs("id");
+const Decl *D = Result.Nodes.getNodeAs("id");
 if (!D || D->isImplicit())
   return;
 NumFoundDecls++;

Modified: cfe/trunk/unittests/AST/StmtPrinterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/StmtPrinterTest.cpp?rev=289543&r1=289542&r2=289543&view=diff
==
--- cfe/trunk/unittests/AST/StmtPrinterTest.cpp (original)
+++ cfe/trunk/unittests/AST/StmtPrinterTest.cpp Tue Dec 13 10:19:34 2016
@@ -45,7 +45,7 @@ public:
   PrintMatch() : NumFoundStmts(0) {}
 
   void run(const MatchFinder::MatchResult &Result) override {
-const Stmt *S = Result.Nodes.getStmtAs("id");
+const Stmt *S = Result.Nodes.getNodeAs("id");
 if (!S)
   return;
 NumFoundStmts++;


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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Jonas, do you need someone to commit the patch for you?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

r289544 - Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.

[Neil Hickey via cfe-commits]

Author: neil.hickey
Date: Tue Dec 13 10:22:50 2016
New Revision: 289544

URL: http://llvm.org/viewvc/llvm-project?rev=289544&view=rev
Log:
Improve handling of floating point literals in OpenCL to only use double 
precision if the target supports fp64.

This change makes sure single-precision floating point types are used if the 
cl_fp64 extension is not supported by the target.

Also removed the check to see whether the OpenCL version is >= 1.2, as this has
been incorporated into the extension setting code.

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


Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/CodeGenOpenCL/fpmath.cl
cfe/trunk/test/SemaOpenCL/extensions.cl

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=289544&r1=289543&r2=289544&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 13 10:22:50 2016
@@ -3748,12 +3748,13 @@ bool Sema::SemaBuiltinFPClassification(C
 diag::err_typecheck_call_invalid_unary_fp)
   << OrigArg->getType() << OrigArg->getSourceRange();
 
-  // If this is an implicit conversion from float -> double, remove it.
+  // If this is an implicit conversion from float -> float or double, remove 
it.
   if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
 Expr *CastArg = Cast->getSubExpr();
 if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
-  assert(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) &&
- "promotion from float to double is the only expected cast here");
+assert((Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
+Cast->getType()->isSpecificBuiltinType(BuiltinType::Float)) &&
+ "promotion from float to either float or double is the only 
expected cast here");
   Cast->setSubExpr(nullptr);
   TheCall->setArg(NumArgs-1, CastArg);
 }

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=289544&r1=289543&r2=289544&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Dec 13 10:22:50 2016
@@ -817,8 +817,16 @@ ExprResult Sema::DefaultArgumentPromotio
   // double.
   const BuiltinType *BTy = Ty->getAs();
   if (BTy && (BTy->getKind() == BuiltinType::Half ||
-  BTy->getKind() == BuiltinType::Float))
-E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
+  BTy->getKind() == BuiltinType::Float)) {
+if (getLangOpts().OpenCL &&
+!(getOpenCLOptions().cl_khr_fp64)) {
+if (BTy->getKind() == BuiltinType::Half) {
+E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
+}
+} else {
+  E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
+}
+  }
 
   // C++ performs lvalue-to-rvalue conversion as a default argument
   // promotion, even on class types, but note:
@@ -3397,10 +3405,13 @@ ExprResult Sema::ActOnNumericConstant(co
 
 if (Ty == Context.DoubleTy) {
   if (getLangOpts().SinglePrecisionConstants) {
-Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
+const BuiltinType *BTy = Ty->getAs();
+if (BTy->getKind() != BuiltinType::Float) {
+  Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
+}
   } else if (getLangOpts().OpenCL &&
- !((getLangOpts().OpenCLVersion >= 120) ||
-   getOpenCLOptions().cl_khr_fp64)) {
+ !(getOpenCLOptions().cl_khr_fp64)) {
+// Impose single-precision float type when cl_khr_fp64 is not enabled.
 Diag(Tok.getLocation(), diag::warn_double_const_requires_fp64);
 Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
   }

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=289544&r1=289543&r2=289544&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Dec 13 10:22:50 2016
@@ -1403,8 +1403,7 @@ static QualType ConvertDeclSpecToType(Ty
   Result = Context.DoubleTy;
 
 if (S.getLangOpts().OpenCL &&
-!((S.getLangOpts().OpenCLVersion >= 120) ||
-  S.getOpenCLOptions().cl_khr_fp64)) {
+!(S.getOpenCLOptions().cl_khr_fp64)) {
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_requires_extension)
   << Result << "cl_khr_fp64";
   declarator.setInvalidType(true);

Modified: cfe/trunk/test/CodeGenOpenCL/fpmath.cl
U

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

tbh. i dont know anything how the clang workflow works :)

do i need to ask someone to commit, or what be my next step?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D24235: [OpenCL] Improve floating point literal handling

[Neil Hickey via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289544: Improve handling of floating point literals in 
OpenCL to only use double… (authored by neil.hickey).

Changed prior to commit:
  https://reviews.llvm.org/D24235?vs=77975&id=81235#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24235

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/CodeGenOpenCL/fpmath.cl
  cfe/trunk/test/SemaOpenCL/extensions.cl

Index: cfe/trunk/test/CodeGenOpenCL/fpmath.cl
===
--- cfe/trunk/test/CodeGenOpenCL/fpmath.cl
+++ cfe/trunk/test/CodeGenOpenCL/fpmath.cl
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown | FileCheck --check-prefix=CHECK --check-prefix=NODIVOPT %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown -cl-fp32-correctly-rounded-divide-sqrt | FileCheck --check-prefix=CHECK --check-prefix=DIVOPT %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -DNOFP64 -cl-std=CL1.2 -triple r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck --check-prefix=CHECK-FLT %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -DFP64 -cl-std=CL1.2 -pedantic | FileCheck --check-prefix=CHECK-DBL %s
 
 typedef __attribute__(( ext_vector_type(4) )) float float4;
 
@@ -21,14 +23,26 @@
   return a / b;
 }
 
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+#if __OPENCL_C_VERSION__ >=120
+void printf(constant char* fmt, ...);
+
+void testdbllit(long *val) {
+  // CHECK-FLT: float 2.00e+01
+  // CHECK-DBL: double 2.00e+01
+  printf("%f", 20.0);
+}
 
+#endif
+
+#ifndef NOFP64
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
 double dpscalardiv(double a, double b) {
   // CHECK: @dpscalardiv
   // CHECK: #[[ATTR]]
   // CHECK-NOT: !fpmath
   return a / b;
 }
+#endif
 
 // CHECK: attributes #[[ATTR]] = {
 // NODIVOPT: "correctly-rounded-divide-sqrt-fp-math"="false"
Index: cfe/trunk/test/SemaOpenCL/extensions.cl
===
--- cfe/trunk/test/SemaOpenCL/extensions.cl
+++ cfe/trunk/test/SemaOpenCL/extensions.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.1
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.2 -DFP64
 
 // Test with a target not supporting fp64.
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
@@ -23,10 +24,16 @@
 
 
 
+#ifdef FP64
+// expected-no-diagnostics
+#endif
+
+#if __OPENCL_C_VERSION__ < 120
 void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
 }
+#endif
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 #ifdef NOFP64
@@ -45,16 +52,19 @@
 #endif
 
   (void) 1.0;
+
 #ifdef NOFP64
-// expected-warning@-2{{double precision constant requires cl_khr_fp64, casting to single precision}}
+// expected-warning@-3{{double precision constant requires cl_khr_fp64, casting to single precision}}
 #endif
 }
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : disable
 #ifdef NOFP64
 // expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
 #endif
 
+#if __OPENCL_C_VERSION__ < 120
 void f3(void) {
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
 }
+#endif
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -817,8 +817,16 @@
   // double.
   const BuiltinType *BTy = Ty->getAs();
   if (BTy && (BTy->getKind() == BuiltinType::Half ||
-  BTy->getKind() == BuiltinType::Float))
-E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
+  BTy->getKind() == BuiltinType::Float)) {
+if (getLangOpts().OpenCL &&
+!(getOpenCLOptions().cl_khr_fp64)) {
+if (BTy->getKind() == BuiltinType::Half) {
+E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
+}
+} else {
+  E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
+}
+  }
 
   // C++ performs lvalue-to-rvalue conversion as a default argument
   // promotion, even on class types, but note:
@@ -3397,10 +3405,13 @@
 
 if (Ty == Context.DoubleTy) {
   if (getLangOpts().SinglePrecisionConstants) {
-Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
+const BuiltinType *BTy = Ty->getAs();
+if (BTy->getKind() != BuiltinType::Float) {
+  Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
+}
   

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D26167#621106, @alexfh wrote:

> Jonas, do you need someone to commit the patch for you?


i think so, yes. can i trigger somehow automatically, that this will go into 
the code base somehow?


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[PATCH] D27424: Add the diagnose_if attribute to clang.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Sema/Overload.h:754
+unsigned NumInlineBytesUsed;
+llvm::AlignedCharArray
 InlineSpace;

I guess you could use "void *" instead of ImplicitConversionSequence for the 
alignment to make it clear that this has the alignment of a pointer type (or 
the larger of alignof(ImplicitConversionSequence) and alignof(DiagnoseIfAttr 
*))?


https://reviews.llvm.org/D27424



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

RE: r286815 - Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.

[Neil Hickey via cfe-commits]

I looked into this, and for C, clang converts the literal to inf in the first 
case.
OpenCL spec doesn't mention anything different so I presume the behaviour 
should match Cs behaviour, which is does with this change.

More exhaustive tests could be implemented to ensure this though.

Neil

> -Original Message-
> From: sca...@apple.com [mailto:sca...@apple.com]
> Sent: 15 November 2016 13:20
> To: Neil Hickey
> Cc: cfe-commits@lists.llvm.org; nd
> Subject: Re: r286815 - Improve handling of floating point literals in OpenCL 
> to
> only use double precision if the target supports fp64.
> 
> The reason these commits bring it up is that I don’t see it clearly documented
> whether:
> 
>   float x = 340282356779733661637539395458142568447.0;
> 
> is interpreted as
> 
>   float x = 340282356779733661637539395458142568447.0f; // x is
> FLT_MAX
> 
> or as
> 
>   float x = (float)(340282356779733661637539395458142568447.0); // x
> is INFINITY
> 
> when the no-fp64 mode is active.  I assume that only one of these is the
> correct behavior, and we should have a regression test that will flag the 
> error
> if it changes.
> 
> – Steve
> 
> > On Nov 15, 2016, at 4:51 AM, Neil Hickey  wrote:
> >
> > It would be valuable to perform that test, if clang tests don't already do
> this, though I'm not sure it should be part of this particular commit as all 
> this
> commit should do is change the casts if the target doesn't support double.
> >
> > Neil
> >
> >> -Original Message-
> >> From: sca...@apple.com [mailto:sca...@apple.com]
> >> Sent: 14 November 2016 16:01
> >> To: Neil Hickey
> >> Cc: cfe-commits@lists.llvm.org
> >> Subject: Re: r286815 - Improve handling of floating point literals in
> >> OpenCL to only use double precision if the target supports fp64.
> >>
> >> Can you add some non-trivial test cases that exercise
> >> double-rounding, especially near the overflow boundary?
> >>
> >> e.g. What is the expected value of x if the target does not support fp64?:
> >>
> >>float x = 340282356779733661637539395458142568447.0;
> >>
> >> – Steve
> >>
> >>> On Nov 14, 2016, at 6:15 AM, Neil Hickey via cfe-commits  >> comm...@lists.llvm.org> wrote:
> >>>
> >>> Author: neil.hickey
> >>> Date: Mon Nov 14 05:15:51 2016
> >>> New Revision: 286815
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=286815&view=rev
> >>> Log:
> >>> Improve handling of floating point literals in OpenCL to only use
> >>> double
> >> precision if the target supports fp64.
> >>>
> >>> This change makes sure single-precision floating point types are
> >>> used if the
> >>> cl_fp64 extension is not supported by the target.
> >>>
> >>> Also removed the check to see whether the OpenCL version is >= 1.2,
> >>> as this has been incorporated into the extension setting code.
> >>>
> >>> Differential Revision: https://reviews.llvm.org/D24235
> >>>
> >>>
> >>> Modified:
> >>>  cfe/trunk/lib/Sema/SemaExpr.cpp
> >>>  cfe/trunk/lib/Sema/SemaType.cpp
> >>>  cfe/trunk/test/CodeGenOpenCL/fpmath.cl
> >>>  cfe/trunk/test/SemaOpenCL/extensions.cl
> >>>
> >>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?
> >>> re v=286815&r1=286814&r2=286815&view=diff
> >>>
> >>
> ==
> >> 
> >>> 
> >>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> >>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Nov 14 05:15:51 2016
> >>> @@ -705,9 +705,13 @@ ExprResult Sema::DefaultLvalueConversion  if
> >>> (getLangOpts().ObjCAutoRefCount &&
> >>> E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
> >>>   Cleanup.setExprNeedsCleanups(true);
> >>> +
> >>> +  ExprResult Res = E;
> >>>
> >>> -  ExprResult Res = ImplicitCastExpr::Create(Context, T,
> >> CK_LValueToRValue, E,
> >>> -nullptr, VK_RValue);
> >>> +  if ( T != E->getType()) {
> >>> +Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
> >>> +   nullptr, VK_RValue);  }
> >>>
> >>> // C11 6.3.2.1p2:
> >>> //   ... if the lvalue has atomic type, the value has the non-atomic 
> >>> version
> >>> @@ -817,8 +821,16 @@ ExprResult Sema::DefaultArgumentPromotio  //
> >>> double.
> >>> const BuiltinType *BTy = Ty->getAs();  if (BTy &&
> >>> (BTy->getKind() == BuiltinType::Half ||
> >>> -  BTy->getKind() == BuiltinType::Float))
> >>> -E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
> >>> +  BTy->getKind() == BuiltinType::Float)) {
> >>> +if (getLangOpts().OpenCL &&
> >>> +!(getOpenCLOptions().cl_khr_fp64)) {
> >>> +if (BTy->getKind() == BuiltinType::Half) {
> >>> +E = ImpCastExprToType(E, Context.FloatTy,
> CK_FloatingCast).get();
> >>> +}
> >>> +} else {
> >>> +  E = ImpCastExprToType(E, Context.DoubleTy,
> CK_FloatingCast).get();
> >>> +}
> >>> +  }
> >>>
> >>> // C++

[PATCH] D27187: [clang-tidy] Do not move parameter if only DeclRefExpr occurs inside of a loop

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG modulo comment.




Comment at: clang-tidy/utils/DeclRefExprUtils.cpp:127
+  match(findAll(declRefExpr(equalsNode(&DeclRef),
+unless(hasAncestor(stmt(anyOf(
+forStmt(), cxxForRangeStmt(), whileStmt(),

flx wrote:
> alexfh wrote:
> > flx wrote:
> > > alexfh wrote:
> > > > How will this work with lambdas / local classes declared inside a loop? 
> > > > Not sure if this case is going to happen in real code, but we'd better 
> > > > be clear about the limitations of the implementation.
> > > Why would this not work? Could you give an example? The way the function 
> > > is written it handles my the use case for identifying when moving the 
> > > parameter is not safe, so I could also just move it into the 
> > > UnnecessaryValueParamCheck.
> > I was thinking about a case where a loop this matcher finds is outside of 
> > the function definition and shouldn't be considered:
> > 
> >   void F() {
> > for (;;) {
> >   struct C {
> > void f(ExpensiveMovableType E) {
> >   auto F = E;
> > }
> >   };
> > }
> >   }
> > 
> This case is not an issue in the check as we're passing f's body statement to 
> the hasLoopStmtAncestor function, so the search is scoped to f's body.
> 
> If you're concerned about the case where someone calls this with  F's body 
> but expects it to be scoped to f I can just move this function into the check 
> and make it an implementation detail.
Making it an implementation detail until other potential users appear sounds 
good.


https://reviews.llvm.org/D27187



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

[clang-tools-extra] r289546 - [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Dec 13 10:38:18 2016
New Revision: 289546

URL: http://llvm.org/viewvc/llvm-project?rev=289546&view=rev
Log:
[Clang-tidy] check for malloc, realloc and free calls

Summary:
This checker flags the use of C-style memory management functionality and notes 
about modern alternatives.
In an earlier revision it tried to autofix some kind of patterns, but that was 
a bad idea. Since memory management can be so widespread in a program, manual 
updating is most likely necessary.
Maybe for special cases, there could be later additions to this basic checker.

This is the first checker I wrote and I never did something with clang (only 
compiling programs). So whenever I missed conventions or did plain retarded 
stuff, feel free to point it out! I am willing to fix them and write a better 
checker.

I hope the patch does work, I never did this either. On a testapply in my 
repository it did, but I am pretty unconfident in my patching skills :)

Reviewers: aaron.ballman, hokein, alexfh, malcolm.parsons

Subscribers: cfe-commits, JDevlieghere, nemanjai, Eugene.Zelenko, Prazek, 
mgorny, modocache

Tags: #clang-tools-extra

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

Patch by Jonas Toth!

Added:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=289546&r1=289545&r2=289546&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Tue Dec 
13 10:38:18 2016
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
+  NoMallocCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=289546&r1=289545&r2=289546&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Tue Dec 13 10:38:18 2016
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "InterfacesGlobalInitCheck.h"
+#include "NoMallocCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
@@ -35,6 +36,7 @@ public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
+CheckFactories.registerCheck("cppcoreguidelines-no-malloc");
 CheckFactories.registerCheck(
 "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
 CheckFactories.registerCheck(

Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp?rev=289546&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp Tue 
Dec 13 10:38:18 2016
@@ -0,0 +1,62 @@
+//===--- NoMallocCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoMallocCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {

[clang-tools-extra] r289547 - Remove trailing whitespace in docs and clang-tidy sources.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Dec 13 10:38:45 2016
New Revision: 289547

URL: http://llvm.org/viewvc/llvm-project?rev=289547&view=rev
Log:
Remove trailing whitespace in docs and clang-tidy sources.

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/trunk/docs/ModularizeUsage.rst
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err34-c.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-transparent-functors.rst
clang-tools-extra/trunk/docs/clang-tidy/index.rst
clang-tools-extra/trunk/test/clang-tidy/boost-use-to-string.cpp

clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-string-constructor.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-swapped-arguments.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-shrink-to-fit.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp
clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=289547&r1=289546&r2=289547&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Tue Dec 
13 10:38:45 2016
@@ -14,7 +14,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Lex/Lexer.h" 
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"

Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp?rev=289547&r1=289546&r2=289547&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp Tue Dec 
13 10:38:45 2016
@@ -14,7 +14,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TokenKinds.h"
-#include "clang/Lex/Lexer.h" 
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"

Modified: clang-tools-extra/trunk/docs/ModularizeUsage.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=289547&r1=289546&r2=289547&view=diff
==
--- clang-tools-extra/trunk/docs/ModularizeUsage.rst (original)
+++ clang-tools-extra/trunk/docs/ModularizeUsage.rst Tue Dec 13 10:38:45 2016
@@ -59,7 +59,7 @@ Modularize Command Line Options
 
   Generate a module map and output it to the given file. See the description
   in :ref:`module-map-generation`.
-  
+
 .. option:: -problem-files-list=
 
   For use only with module map assistant. Input list of files that

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=289547&r1=289546&r2=289547&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Dec 13 10:38:45 2016
@@ -81,7 +81,7 @@ Improvements to clang-tidy
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
-- New `cppcoreguidelines-no-malloc 
+- New `cppcoreguidelines-no-malloc
   
`_
 check
   warns if C-style memory management is used and suggests the use of RAII.
 

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err34-c.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err34-c.rst?rev=289547&r1=289546&r2=289547&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-err34-c.rst (original)
+++ clang-

[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons planned changes to this revision.
malcolm.parsons added inline comments.



Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:21
+
+static StringRef getValueInit(const CXXCtorInitializer *Init) {
+  switch (Init->getInit()->getType()->getScalarTypeKind()) {

alexfh wrote:
> The function name doesn't make it clear that the value is returned for 
> zero-initialization.
`getValueOfValueInit`?


https://reviews.llvm.org/D26750



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

[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Sema/Sema.h:8081
+  /// \brief Set current OpenCL extensions for a type which can only be used
+  /// when these OpenCL extensions are enabled. If current OpenCL Extsion is
+  /// empty, do nothing.

Extsion -> extension



Comment at: include/clang/Sema/Sema.h:8120
 
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.

or declaration -> or a declaration



Comment at: include/clang/Sema/Sema.h:8121
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.
+  /// \param D type or declaration to be checked.

is disabled -> being disabled



Comment at: lib/Serialization/ASTReader.cpp:3167
 case OPENCL_EXTENSIONS:
-  // Later tables overwrite earlier ones.
-  OpenCLExtensions.swap(Record);

Btw, OpenCLTypeExtMap and OpenCLTypeDeclMap don't have to be serialized?



Comment at: test/Parser/opencl-atomics-cl20.cl:51
 // expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)') 
requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-27 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
-// expected-error-re@-28 {{use of type 'atomic_uintptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
-// expected-error-re@-29 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') 
requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') 
requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-30 {{use of type 'atomic_ptrdiff_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-31 {{use of type 'atomic_ptrdiff_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
+#if __LP64__
+// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}

yaxunl wrote:
> Anastasia wrote:
> > Why this change?
> atomic_intptr_t etc. requires cl_khr_int64_extended_atomics only on 64 bit 
> platforms.
> 
> This is a bug which was fixed by this patch.
The spec says:
"If the device address space is 64-bits, the data types atomic_intptr_t, 
atomic_uintptr_t,
atomic_size_t and atomic_ptrdiff_t are supported if the 
cl_khr_int64_base_atomics and
cl_khr_int64_extended_atomics extensions are supported."

This seems to be the same as long and double?


https://reviews.llvm.org/D21698



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

[clang-tools-extra] r289549 - Fix sphinx build.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Dec 13 10:49:10 2016
New Revision: 289549

URL: http://llvm.org/viewvc/llvm-project?rev=289549&view=rev
Log:
Fix sphinx build.

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst?rev=289549&r1=289548&r2=289549&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-no-malloc.rst 
Tue Dec 13 10:49:10 2016
@@ -7,7 +7,7 @@ This check handles C-Style memory manage
 ``calloc()`` and ``free()``. It warns about its use and tries to suggest the 
use
 of an appropriate RAII object.
 See `C++ Core Guidelines
-
+`.
 
 There is no attempt made to provide fixit hints, since manual resource 
management isn't
 easily transformed automatically into RAII.


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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Committed in r289546.


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

r289552 - Fixing build failure by adding triple option to new test condition.

[Neil Hickey via cfe-commits]

Author: neil.hickey
Date: Tue Dec 13 11:04:33 2016
New Revision: 289552

URL: http://llvm.org/viewvc/llvm-project?rev=289552&view=rev
Log:
Fixing build failure by adding triple option to new test condition.

Adding -triple option to ensure target supports double for fpmath test.


Modified:
cfe/trunk/test/CodeGenOpenCL/fpmath.cl

Modified: cfe/trunk/test/CodeGenOpenCL/fpmath.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/fpmath.cl?rev=289552&r1=289551&r2=289552&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/fpmath.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/fpmath.cl Tue Dec 13 11:04:33 2016
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown | FileCheck 
--check-prefix=CHECK --check-prefix=NODIVOPT %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown 
-cl-fp32-correctly-rounded-divide-sqrt | FileCheck --check-prefix=CHECK 
--check-prefix=DIVOPT %s
 // RUN: %clang_cc1 %s -emit-llvm -o - -DNOFP64 -cl-std=CL1.2 -triple 
r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck 
--check-prefix=CHECK-FLT %s
-// RUN: %clang_cc1 %s -emit-llvm -o - -DFP64 -cl-std=CL1.2 -pedantic | 
FileCheck --check-prefix=CHECK-DBL %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -DFP64 -cl-std=CL1.2 -triple 
spir-unknown-unknown -pedantic | FileCheck --check-prefix=CHECK-DBL %s
 
 typedef __attribute__(( ext_vector_type(4) )) float float4;
 


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

r289553 - Update for clang after llvm::StringLiteral.

[Zachary Turner via cfe-commits]

Author: zturner
Date: Tue Dec 13 11:10:16 2016
New Revision: 289553

URL: http://llvm.org/viewvc/llvm-project?rev=289553&view=rev
Log:
Update for clang after llvm::StringLiteral.

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

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=289553&r1=289552&r2=289553&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Dec 13 11:10:16 2016
@@ -4189,7 +4189,7 @@ static Value *EmitSpecialRegisterBuiltin
 
   if (SysReg.empty()) {
 const Expr *SysRegStrExpr = E->getArg(0)->IgnoreParenCasts();
-SysReg = cast(SysRegStrExpr)->getString();
+SysReg = cast(SysRegStrExpr)->getString();
   }
 
   llvm::Metadata *Ops[] = { llvm::MDString::get(Context, SysReg) };


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

[PATCH] D27717: [analyzer] Suppress a leak false positive in Qt's QObject::connectImpl()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin.
NoQ added a subscriber: cfe-commits.

A quick fix to address the false positive posted by Tiago Macarios in the 
mailing lists: http://lists.llvm.org/pipermail/cfe-dev/2016-December/051738.html

MallocChecker processes pointer escapes heuristically rather than regularly. 
Force it to treat the pointers passed to `connectImpl()` as escaping.

I dislike a few things about this patch - we're patching against implementation 
details and hardcoding names of private methods in an external library. I don't 
see a significantly better solution within the current approach though.

See also https://reviews.llvm.org/D27599 - a similar hack for another function.


https://reviews.llvm.org/D27717

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/Inputs/qt-simulator.h
  test/Analysis/qt_malloc.cpp


Index: test/Analysis/qt_malloc.cpp
===
--- test/Analysis/qt_malloc.cpp
+++ test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: test/Analysis/Inputs/qt-simulator.h
===
--- test/Analysis/Inputs/qt-simulator.h
+++ test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2579,6 +2579,11 @@
 return true;
   }
 
+  if (FName == "connectImpl" &&
+  FD->getQualifiedNameAsString() == "QObject::connectImpl") {
+return true;
+  }
+
   // Handle cases where we know a buffer's /address/ can escape.
   // Note that the above checks handle some special cases where we know that
   // even though the address escapes, it's still our responsibility to free the


Index: test/Analysis/qt_malloc.cpp
===
--- test/Analysis/qt_malloc.cpp
+++ test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: test/Analysis/Inputs/qt-simulator.h
===
--- test/Analysis/Inputs/qt-simulator.h
+++ test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp

r289554 - [analyzer] Detect ObjC properties that are both (copy) and Mutable.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Dec 13 11:19:18 2016
New Revision: 289554

URL: http://llvm.org/viewvc/llvm-project?rev=289554&view=rev
Log:
[analyzer] Detect ObjC properties that are both (copy) and Mutable.

When an Objective-C property has a (copy) attribute, the default setter
for this property performs a -copy on the object assigned.

Calling -copy on a mutable NS object such as NSMutableString etc.
produces an immutable object, NSString in our example.
Hence the getter becomes type-incorrect.

rdar://problem/21022397

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

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=289554&r1=289553&r2=289554&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Tue Dec 13 
11:19:18 2016
@@ -480,20 +480,21 @@ def CStringNotNullTerm : Checker<"NotNul
 let ParentPackage = OSX in {
 
 def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
-  InPackage,
   HelpText<"Check for erroneous conversions of objects representing numbers 
into numbers">,
   DescFile<"NumberObjectConversionChecker.cpp">;
 
 def MacOSXAPIChecker : Checker<"API">,
-  InPackage,
   HelpText<"Check for proper uses of various Apple APIs">,
   DescFile<"MacOSXAPIChecker.cpp">;
 
 def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">,
-  InPackage,
   HelpText<"Check for proper uses of Secure Keychain APIs">,
   DescFile<"MacOSKeychainAPIChecker.cpp">;
 
+def ObjCPropertyChecker : Checker<"ObjCProperty">,
+  HelpText<"Check for proper uses of Objective-C properties">,
+  DescFile<"ObjCPropertyChecker.cpp">;
+
 } // end "osx"
 
 let ParentPackage = Cocoa in {

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=289554&r1=289553&r2=289554&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Tue Dec 13 11:19:18 
2016
@@ -59,6 +59,7 @@ add_clang_library(clangStaticAnalyzerChe
   ObjCContainersASTChecker.cpp
   ObjCContainersChecker.cpp
   ObjCMissingSuperCallChecker.cpp
+  ObjCPropertyChecker.cpp
   ObjCSelfInitChecker.cpp
   ObjCSuperDeallocChecker.cpp
   ObjCUnusedIVarsChecker.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp?rev=289554&view=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp Tue Dec 13 
11:19:18 2016
@@ -0,0 +1,82 @@
+//==- ObjCPropertyChecker.cpp - Check ObjC properties *- C++ 
-*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This checker finds issues with Objective-C properties.
+//  Currently finds only one kind of issue:
+//  - Find synthesized properties with copy attribute of mutable NS collection
+//types. Calling -copy on such collections produces an immutable copy,
+//which contradicts the type of the property.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class ObjCPropertyChecker
+: public Checker> {
+  void checkCopyMutable(const ObjCPropertyDecl *D, BugReporter &BR) const;
+
+public:
+  void checkASTDecl(const ObjCPropertyDecl *D, AnalysisManager &Mgr,
+BugReporter &BR) const;
+};
+} // end anonymous namespace.
+
+void ObjCPropertyChecker::checkASTDecl(const ObjCPropertyDecl *D,
+   AnalysisManager &Mgr,
+   BugReporter &BR) const {
+  checkCopyMutable(D, BR);
+}
+
+void ObjCPropertyChecker::checkCopyMutable(const ObjCPropertyDecl *D,
+   BugReporter &BR) const {
+  if (D->isReadOnly() || D->getSetterKind() != ObjCPro

[PATCH] D27535: [analyzer] Add ObjCPropertyChecker - check for autosynthesized copy-properties of mutable types.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289554: [analyzer] Detect ObjC properties that are both 
(copy) and Mutable. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D27535?vs=81083&id=81240#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27535

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
  cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m

Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -480,20 +480,21 @@
 let ParentPackage = OSX in {
 
 def NumberObjectConversionChecker : Checker<"NumberObjectConversion">,
-  InPackage,
   HelpText<"Check for erroneous conversions of objects representing numbers into numbers">,
   DescFile<"NumberObjectConversionChecker.cpp">;
 
 def MacOSXAPIChecker : Checker<"API">,
-  InPackage,
   HelpText<"Check for proper uses of various Apple APIs">,
   DescFile<"MacOSXAPIChecker.cpp">;
 
 def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">,
-  InPackage,
   HelpText<"Check for proper uses of Secure Keychain APIs">,
   DescFile<"MacOSKeychainAPIChecker.cpp">;
 
+def ObjCPropertyChecker : Checker<"ObjCProperty">,
+  HelpText<"Check for proper uses of Objective-C properties">,
+  DescFile<"ObjCPropertyChecker.cpp">;
+
 } // end "osx"
 
 let ParentPackage = Cocoa in {
Index: cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m
===
--- cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m
+++ cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -w -fblocks -analyze -analyzer-checker=osx.ObjCProperty %s -verify
+
+#include "Inputs/system-header-simulator-objc.h"
+
+@interface I : NSObject {
+  NSMutableString *_mutableExplicitStr;
+  NSMutableString *_trulyMutableStr;
+  NSMutableString *_trulyMutableExplicitStr;
+}
+@property(copy) NSString *str; // no-warning
+@property(copy) NSMutableString *mutableStr; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@property(copy) NSMutableString *mutableExplicitStr; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@property(copy, readonly) NSMutableString *mutableReadonlyStr; // no-warning
+@property(copy, readonly) NSMutableString *mutableReadonlyStrOverriddenInChild; // no-warning
+@property(copy, readonly) NSMutableString *mutableReadonlyStrOverriddenInCategory; // no-warning
+@property(copy) NSMutableString *trulyMutableStr; // no-warning
+@property(copy) NSMutableString *trulyMutableExplicitStr; // no-warning
+@property(copy) NSMutableString *trulyMutableStrWithSynthesizedStorage; // no-warning
+@end
+
+@interface I () {}
+@property(copy) NSMutableString *mutableStrInCategory; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@property (copy, readwrite) NSMutableString *mutableReadonlyStrOverriddenInCategory; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@end
+
+@implementation I
+@synthesize mutableExplicitStr = _mutableExplicitStr;
+- (NSMutableString *)trulyMutableStr {
+  return _trulyMutableStr;
+}
+- (void)setTrulyMutableStr: (NSMutableString *) S {
+  _trulyMutableStr = [S mutableCopy];
+}
+@dynamic trulyMutableExplicitStr;
+- (NSMutableString *)trulyMutableExplicitStr {
+  return _trulyMutableExplicitStr;
+}
+- (void)setTrulyMutableExplicitStr: (NSMutableString *) S {
+  _trulyMutableExplicitStr = [S mutableCopy];
+}
+@synthesize trulyMutableStrWithSynthesizedStorage;
+- (NSMutableString *)trulyMutableStrWithSynthesizedStorage {
+  return trulyMutableStrWithSynthesizedStorage;
+}
+- (void)setTrulyMutableStrWithSynthesizedStorage: (NSMutableString *) S {
+  trulyMutableStrWithSynthesizedStorage = [S mutableCopy];
+}
+@end
+
+@interface J : I {}
+@property (copy, readwrite) NSMutableString *mutableReadonlyStrOverriddenInChild; // expected-warning{{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@end
+
+@implementation J
+@end
+
+// If we do not see the implementation then we do not want to warn,
+// because we may miss a user-defined setter that works correctly.
+@interface IWithoutImpl : NSObject {}
+@property(copy) NSMutableString *mutableStr; // no-warning
+@end
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- cfe/

[PATCH] D27717: [analyzer] Suppress a leak false positive in Qt's QObject::connectImpl()

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2577
 
   if (FName == "postEvent" &&
   FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {

Sorry for disturbing you for code which is not yours but this line looks 
exactly like the line upper and seems to be a dead/useless code.


https://reviews.llvm.org/D27717



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

[PATCH] D27655: Fix modernize-deprecated-headers clang-tidy warnings

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Yes, IWYU is useful tool, but please note that it's not ideal, so will be good 
idea to check proposed changes manually.

You could build IWYU in-tree with Clang or Clang extra tools.


https://reviews.llvm.org/D27655



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

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: docs/clang-tidy/checks/misc-string-compare.rst:12
+zero depending on the lexicographical relationship between the strings 
compared. 
+If an equality or inequality check can suffice, that is recommended.
+

alexfh wrote:
> The documentation doesn't explain why is using `compare` for equality 
> comparison should be avoided. Maybe `that is recommended to avoid the risk of 
> incorrect interpretation of the return value and simplify the code`?
Also, a string equality check can be faster than compare as it can exit early 
when the strings aren't the same length.


https://reviews.llvm.org/D27210



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

r289567 - __uuidof() and declspec(uuid("...")) should be allowed on enumeration types

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Dec 13 12:58:09 2016
New Revision: 289567

URL: http://llvm.org/viewvc/llvm-project?rev=289567&view=rev
Log:
__uuidof() and declspec(uuid("...")) should be allowed on enumeration types

Although not specifically mentioned in the documentation, MSVC accepts
__uuidof(…) and declspec(uuid("…")) attributes on enumeration types in
addition to structs/classes. This is meaningful, as such types *do* have
associated UUIDs in ActiveX typelibs, and such attributes are included
by default in the wrappers generated by their #import construct, so they
are not particularly unusual.

clang currently rejects the declspec with a –Wignored-attributes
warning, and errors on __uuidof() with “cannot call operator __uuidof on
a type with no GUID” (because it rejected the uuid attribute, and
therefore finds no value). This is causing problems for us while trying
to use clang-tidy on a codebase that makes heavy use of ActiveX.

I believe I have found the relevant places to add this functionality,
this patch adds this case to clang’s implementation of these MS
extensions.  patch is against r285994 (or actually the git mirror
80464680ce).

Both include an update to test/Parser/MicrosoftExtensions.cpp to
exercise the new functionality.

This is my first time contributing to LLVM, so if I’ve missed anything
else needed to prepare this for review just let me know!

__uuidof: https://msdn.microsoft.com/en-us/library/zaah6a61.aspx
declspec(uuid("…")): https://msdn.microsoft.com/en-us/library/3b6wkewa.aspx
 #import: https://msdn.microsoft.com/en-us/library/8etzzkb6.aspx

Reviewers: aaron.ballman, majnemer, rnk

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/AttributeList.h
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/Parser/MicrosoftExtensions.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=289567&r1=289566&r2=289567&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Dec 13 12:58:09 2016
@@ -1618,7 +1618,9 @@ def Used : InheritableAttr {
 def Uuid : InheritableAttr {
   let Spellings = [Declspec<"uuid">, Microsoft<"uuid">];
   let Args = [StringArgument<"Guid">];
-//  let Subjects = SubjectList<[CXXRecord]>;
+  let Subjects = SubjectList<[Record, Enum], WarnDiag, "ExpectedEnumOrClass">;
+  // FIXME: Allow expressing logical AND for LangOpts. Our condition should be:
+  // CPlusPlus && (MicrosoftExt || Borland)
   let LangOpts = [MicrosoftExt, Borland];
   let Documentation = [Undocumented];
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=289567&r1=289566&r2=289567&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec 13 12:58:09 
2016
@@ -2650,7 +2650,8 @@ def warn_attribute_wrong_decl_type : War
   "|variables, enums, fields and typedefs"
   "|functions, methods, enums, and classes"
   "|structs, classes, variables, functions, and inline namespaces"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, 
and non-static data members}1">,
+  "|variables, functions, methods, types, enumerations, enumerators, labels, 
and non-static data members"
+  "|classes and enumerations}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=289567&r1=289566&r2=289567&view=diff
==
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Dec 13 12:58:09 2016
@@ -925,7 +925,8 @@ enum AttributeDeclKind {
   ExpectedVariableEnumFieldOrTypedef,
   ExpectedFunctionMethodEnumOrClass,
   ExpectedStructClassVariableFunctionOrInlineNamespace,
-  ExpectedForMaybeUnused
+  ExpectedForMaybeUnused,
+  ExpectedEnumOrClass,
 };
 
 }  // end namespace clang

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=289567&r1=289566&r2=289567&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Dec 13 12:58:09 2016
@@ -4666,12 +4666,6 @@ static void handleUuidAttr(Sema &S, Decl
 return;
   }
 
-  

[PATCH] D26846: __uuidof() and declspec(uuid("...")) should be allowed on enumeration types

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

FYI I had to make a few changes so that we still error out on __uuid in C mode.


Repository:
  rL LLVM

https://reviews.llvm.org/D26846



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

[PATCH] D26846: __uuidof() and declspec(uuid("...")) should be allowed on enumeration types

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289567: __uuidof() and declspec(uuid("...")) should be 
allowed on enumeration types (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D26846?vs=78569&id=81258#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26846

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/AttributeList.h
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/test/Parser/MicrosoftExtensions.cpp

Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -4666,12 +4666,6 @@
 return;
   }
 
-  if (!isa(D)) {
-S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << Attr.getName() << ExpectedClass;
-return;
-  }
-
   StringRef StrRef;
   SourceLocation LiteralLoc;
   if (!S.checkStringLiteralArgumentAttr(Attr, 0, StrRef, &LiteralLoc))
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -520,17 +520,17 @@
   else if (QT->isArrayType())
 Ty = Ty->getBaseElementTypeUnsafe();
 
-  const auto *RD = Ty->getAsCXXRecordDecl();
-  if (!RD)
+  const auto *TD = Ty->getAsTagDecl();
+  if (!TD)
 return;
 
-  if (const auto *Uuid = RD->getMostRecentDecl()->getAttr()) {
+  if (const auto *Uuid = TD->getMostRecentDecl()->getAttr()) {
 UuidAttrs.insert(Uuid);
 return;
   }
 
   // __uuidof can grab UUIDs from template arguments.
-  if (const auto *CTSD = dyn_cast(RD)) {
+  if (const auto *CTSD = dyn_cast(TD)) {
 const TemplateArgumentList &TAL = CTSD->getTemplateArgs();
 for (const TemplateArgument &TA : TAL.asArray()) {
   const UuidAttr *UuidForTA = nullptr;
Index: cfe/trunk/include/clang/Sema/AttributeList.h
===
--- cfe/trunk/include/clang/Sema/AttributeList.h
+++ cfe/trunk/include/clang/Sema/AttributeList.h
@@ -925,7 +925,8 @@
   ExpectedVariableEnumFieldOrTypedef,
   ExpectedFunctionMethodEnumOrClass,
   ExpectedStructClassVariableFunctionOrInlineNamespace,
-  ExpectedForMaybeUnused
+  ExpectedForMaybeUnused,
+  ExpectedEnumOrClass,
 };
 
 }  // end namespace clang
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2650,7 +2650,8 @@
   "|variables, enums, fields and typedefs"
   "|functions, methods, enums, and classes"
   "|structs, classes, variables, functions, and inline namespaces"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members}1">,
+  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members"
+  "|classes and enumerations}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -1618,7 +1618,9 @@
 def Uuid : InheritableAttr {
   let Spellings = [Declspec<"uuid">, Microsoft<"uuid">];
   let Args = [StringArgument<"Guid">];
-//  let Subjects = SubjectList<[CXXRecord]>;
+  let Subjects = SubjectList<[Record, Enum], WarnDiag, "ExpectedEnumOrClass">;
+  // FIXME: Allow expressing logical AND for LangOpts. Our condition should be:
+  // CPlusPlus && (MicrosoftExt || Borland)
   let LangOpts = [MicrosoftExt, Borland];
   let Documentation = [Undocumented];
 }
Index: cfe/trunk/test/Parser/MicrosoftExtensions.cpp
===
--- cfe/trunk/test/Parser/MicrosoftExtensions.cpp
+++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp
@@ -65,6 +65,12 @@
 struct
 struct_with_uuid2 {} ;
 
+enum __declspec(uuid("00A0---C000-0046"))
+enum_with_uuid { };
+enum enum_without_uuid { };
+
+int __declspec(uuid("00A0---C000-0046")) inappropriate_uuid; // expected-warning {{'uuid' attribute only applies to classes and enumerations}}
+
 int uuid_sema_test()
 {
struct_with_uuid var_with_uuid[1];
@@ -81,6 +87,15 @@
__uuidof(const struct_with_uuid[1][1]);
__uuidof(const struct_with_uuid*[1][1]); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
 
+   __uuidof(enum_with_uuid);
+   __uuidof(enum_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
+   __uuidof(enum_with_uuid*);
+   __uuidof(enum_without_uuid*); // expected-error {{cannot call operator __uuid

[PATCH] D27424: Add the diagnose_if attribute to clang.

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv updated this revision to Diff 81259.
george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.

Addressed alignment comment. Thanks! (Late parsing is still to-be-done)


https://reviews.llvm.org/D27424

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Overload.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/Sema/diagnose_if.c
  test/SemaCXX/diagnose_if.cpp

Index: test/SemaCXX/diagnose_if.cpp
===
--- /dev/null
+++ test/SemaCXX/diagnose_if.cpp
@@ -0,0 +1,352 @@
+// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++11
+
+#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__)))
+
+namespace type_dependent {
+template 
+void neverok() _diagnose_if(!T(), "oh no", "error") {} // expected-note 4{{from diagnose_if}}
+
+template 
+void alwaysok() _diagnose_if(T(), "oh no", "error") {}
+
+template 
+void alwayswarn() _diagnose_if(!T(), "oh no", "warning") {} // expected-note 4{{from diagnose_if}}
+
+template 
+void neverwarn() _diagnose_if(T(), "oh no", "warning") {}
+
+void runAll() {
+  alwaysok();
+  alwaysok();
+
+  {
+void (*pok)() = alwaysok;
+pok = &alwaysok;
+  }
+
+  neverok(); // expected-error{{oh no}}
+  neverok(); // expected-error{{oh no}}
+
+  {
+void (*pok)() = neverok; // expected-error{{oh no}}
+  }
+  {
+void (*pok)();
+pok = &neverok; // expected-error{{oh no}}
+  }
+
+  alwayswarn(); // expected-warning{{oh no}}
+  alwayswarn(); // expected-warning{{oh no}}
+  {
+void (*pok)() = alwayswarn; // expected-warning{{oh no}}
+pok = &alwayswarn; // expected-warning{{oh no}}
+  }
+
+  neverwarn();
+  neverwarn();
+  {
+void (*pok)() = neverwarn;
+pok = &neverwarn;
+  }
+}
+
+template 
+void errorIf(T a) _diagnose_if(T() != a, "oh no", "error") {} // expected-note {{candidate disabled: oh no}}
+
+template 
+void warnIf(T a) _diagnose_if(T() != a, "oh no", "warning") {} // expected-note {{from diagnose_if}}
+
+void runIf() {
+  errorIf(0);
+  errorIf(1); // expected-error{{call to unavailable function}}
+
+  warnIf(0);
+  warnIf(1); // expected-warning{{oh no}}
+}
+}
+
+namespace value_dependent {
+template 
+void neverok() _diagnose_if(N == 0 || N != 0, "oh no", "error") {} // expected-note 4{{from diagnose_if}}
+
+template 
+void alwaysok() _diagnose_if(N == 0 && N != 0, "oh no", "error") {}
+
+template 
+void alwayswarn() _diagnose_if(N == 0 || N != 0, "oh no", "warning") {} // expected-note 4{{from diagnose_if}}
+
+template 
+void neverwarn() _diagnose_if(N == 0 && N != 0, "oh no", "warning") {}
+
+void runAll() {
+  alwaysok<0>();
+  alwaysok<1>();
+
+  {
+void (*pok)() = alwaysok<0>;
+pok = &alwaysok<0>;
+  }
+
+  neverok<0>(); // expected-error{{oh no}}
+  neverok<1>(); // expected-error{{oh no}}
+
+  {
+void (*pok)() = neverok<0>; // expected-error{{oh no}}
+  }
+  {
+void (*pok)();
+pok = &neverok<0>; // expected-error{{oh no}}
+  }
+
+  alwayswarn<0>(); // expected-warning{{oh no}}
+  alwayswarn<1>(); // expected-warning{{oh no}}
+  {
+void (*pok)() = alwayswarn<0>; // expected-warning{{oh no}}
+pok = &alwayswarn<0>; // expected-warning{{oh no}}
+  }
+
+  neverwarn<0>();
+  neverwarn<1>();
+  {
+void (*pok)() = neverwarn<0>;
+pok = &neverwarn<0>;
+  }
+}
+
+template 
+void errorIf(int a) _diagnose_if(N != a, "oh no", "error") {} // expected-note {{candidate disabled: oh no}}
+
+template 
+void warnIf(int a) _diagnose_if(N != a, "oh no", "warning") {} // expected-note {{from diagnose_if}}
+
+void runIf() {
+  errorIf<0>(0);
+  errorIf<0>(1); // expected-error{{call to unavailable function}}
+
+  warnIf<0>(0);
+  warnIf<0>(1); // expected-warning{{oh no}}
+}
+}
+
+namespace no_overload_interaction {
+void foo(int) _diagnose_if(1, "oh no", "error"); // expected-note{{from diagnose_if}}
+void foo(short);
+
+void bar(int);
+void bar(short) _diagnose_if(1, "oh no", "error");
+
+void fooArg(int a) _diagnose_if(a, "oh no", "error"); // expected-note{{candidate disabled: oh no}}
+void fooArg(short); // expected-note{{candidate function}}
+
+void barArg(int);
+void barArg(short a) _diagnose_if(a, "oh no", "error");
+
+void runAll() {
+  foo(1); // expected-error{{oh no}}
+  bar(1);
+
+  fooArg(1); // expected-error{{call to unavailable function}}
+  barArg(1);
+
+  auto p = foo; // expected-error{{incompatible initializer of type ''}}
+}
+}
+
+// We have some inline space craziness going on. This is meant to go over that
+// in hopes of hitting some edge cases. At the time of writing, our inline
+// buffer can support 16 DiagnoseIfAttr*s. If a function only has one attr, no
+// allocation is done.
+namespace

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via Phabricator via cfe-commits]

madsravn updated this revision to Diff 81260.
madsravn added a comment.

Updated matcher.

Made fixits for some cases

Updated the documentation.


https://reviews.llvm.org/D27210

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/StringCompareCheck.cpp
  clang-tidy/misc/StringCompareCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-string-compare.rst
  test/clang-tidy/misc-string-compare.cpp

Index: test/clang-tidy/misc-string-compare.cpp
===
--- test/clang-tidy/misc-string-compare.cpp
+++ test/clang-tidy/misc-string-compare.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s misc-string-compare %t -- -- -std=c++11
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template , typename A = std::allocator>
+class basic_string {
+public:
+  basic_string();
+  basic_string(const C *, unsigned int size);
+  int compare(const basic_string &str) const;
+  int compare(const C *) const;
+  int compare(int, int, const basic_string &str) const;
+  bool empty();
+};
+bool operator==(const basic_string &lhs, const basic_string &rhs);
+bool operator!=(const basic_string &lhs, const basic_string &rhs);
+typedef basic_string string;
+}
+
+void func(bool b);
+
+void Test() {
+  std::string str1("a", 1);
+  std::string str2("b", 1);
+
+  if (str1.compare(str2)) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]
+  if (!str1.compare(str2)) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:8: warning: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]
+  if (str1.compare(str2) == 0) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str1 == str2) {
+  if (str1.compare(str2) != 0) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str1 != str2) {
+  if (str1.compare("foo") == 0) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str1 == "foo") {
+  if (0 == str1.compare(str2)) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str2 == str1) {
+  if (0 != str1.compare(str2)) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str2 != str1) {
+  func(str1.compare(str2));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: do not use 'compare' to test equality of strings;
+  if (str2.empty() || str1.compare(str2) != 0) {
+  }
+  // CHECK-MESSAGES: [[@LINE-2]]:23: warning: do not use 'compare' to test equality of strings;
+  // CHECK-FIXES: if (str2.empty() || str1 != str2) {
+}
+
+void Valid() {
+  std::string str1("a", 1);
+  std::string str2("b", 1);
+  if (str1 == str2) {
+  }
+  if (str1 != str2) {
+  }
+  if (str1.compare(str2) == 1) {
+  }
+  if (str1.compare(str2) == str1.compare(str2)) {
+  }
+  if (0 == 0) {
+  }
+  if (1 == str1.compare(str2)) {
+  }
+  if (str1.compare(str2) > 0) {
+  }
+  if (str1.compare(1, 3, str2)) {
+  }
+}
Index: docs/clang-tidy/checks/misc-string-compare.rst
===
--- docs/clang-tidy/checks/misc-string-compare.rst
+++ docs/clang-tidy/checks/misc-string-compare.rst
@@ -0,0 +1,54 @@
+.. title:: clang-tidy - misc-string-compare
+
+misc-string-compare
+===
+
+Finds string comparisons using the compare method.
+
+A common mistake is to use the string's ``compare`` method instead of using the 
+equality or inequality operators. The compare method is intended for sorting
+functions and thus returns a negative number, a positive number or 
+zero depending on the lexicographical relationship between the strings compared. 
+If an equality or inequality check can suffice, that is recommended. This is 
+recommended to avoid the risk of incorrect interpretation of the return value
+and to simplify the code. The string equality and inequality operators can
+also be faster than the ``compare`` method due to early termination.
+
+Examples:
+
+.. code-block:: c++
+
+  std::string str1{"a"};
+  std::string str2{"b"};
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2)) {
+  }
+
+  // use str1 == str2 instead.
+  if (!str1.compare(str2)) {
+  }
+
+  // use str1 == str2 instead.
+  if (str1.compare(str2) == 0) {
+  }
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2) != 0) {
+  }
+
+  // use str1 == str2 instead.
+  if (0 == str1.compare(str2)) {
+  }
+
+  // use str1 != str2 instead.
+  if (0 != str1.compare(str2)) {
+  }
+
+  // Use str1 == "foo" instead.
+  if (str1.compare("foo") == 0) {
+  }
+
+The above 

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

[Mark Lodato via Phabricator via cfe-commits]

lodato resigned from this revision.
lodato removed a reviewer: lodato.
lodato added a comment.

I know nothing about the C++ code. I only know the git-clang-format script.


https://reviews.llvm.org/D27377



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

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 81264.
pcc added a comment.

- Address review comments


https://reviews.llvm.org/D22296

Files:
  clang/include/clang/AST/VTableBuilder.h
  clang/include/clang/Basic/LLVM.h
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/CodeGen/CGCXX.cpp
  clang/lib/CodeGen/CGVTT.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/PR26569.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/ctor-dtor-alias.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-rtti.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/invariant.group-for-vptrs.cpp
  clang/test/CodeGenCXX/key-function-vtable.cpp
  clang/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-abi-extern-template.cpp
  clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-abi-structors.cpp
  clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
  clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
  clang/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-interface.cpp
  clang/test/CodeGenCXX/microsoft-no-rtti-data.cpp
  clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
  clang/test/CodeGenCXX/strict-vtable-pointers.cpp
  clang/test/CodeGenCXX/vtable-align.cpp
  clang/test/CodeGenCXX/vtable-assume-load.cpp
  clang/test/CodeGenCXX/vtable-pointer-initialization.cpp
  clang/test/CodeGenCXX/vtt-layout.cpp

Index: clang/test/CodeGenCXX/vtt-layout.cpp
===
--- clang/test/CodeGenCXX/vtt-layout.cpp
+++ clang/test/CodeGenCXX/vtt-layout.cpp
@@ -78,11 +78,12 @@
   }
 }
 
-// CHECK: @_ZTTN5Test11BE = unnamed_addr constant [1 x i8*] [i8* bitcast (i8** getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTVN5Test11BE, i32 0, i32 3) to i8*)]
-// CHECK: @_ZTVN5Test51AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test51AE to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*), i8* bitcast (void (%"struct.Test5::A"*)* @_ZN5Test51A6anchorEv to i8*)]
-// CHECK: @_ZTVN5Test61AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test61AE to i8*), i8* bitcast (void ()* @__cxa_deleted_virtual to i8*), i8* bitcast (void (%"struct.Test6::A"*)* @_ZN5Test61A6anchorEv to i8*)]
-// CHECK: @_ZTTN5Test21CE = linkonce_odr unnamed_addr constant [2 x i8*] [i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*), i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*)] 
-// CHECK: @_ZTTN5Test31DE = linkonce_odr unnamed_addr constant [13 x i8*] [i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 5) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 10) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 13) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 15) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i64 1, i32 0) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 6) to i8*)] 
-// CHECK: @_ZTTN5Test41DE = linkonce_odr unnamed_addr constant [19 x i8*] [i8* bitcast (i8** getelementptr inbounds ([25 x i8*], [25 x i8*]* @_ZTVN5Test41DE, i32 0, i

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc marked 3 inline comments as done.
pcc added inline comments.



Comment at: clang/include/clang/AST/VTableBuilder.h:255
+operator ArrayRef() const { return {data(), size()}; };
+  };
+

rjmccall wrote:
> Maybe this ought to be in LLVM as OwnedArrayRef?  And the more minimal 
> implementation approach would be to inherit from MutableArrayRef and just 
> add a destructor and a move constructor.
> 
> The implicit conversion to ArrayRef is dangerous, but only in ways that 
> ArrayRef is already dangerous.
Good suggestions -- sent out D27723.


https://reviews.llvm.org/D22296



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

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:297
 case tok::kw_default:
+  if (Style.Language != FormatStyle::LK_Java || !Line->MustBeDeclaration) {
+if (!SwitchLabelEncountered &&

Same as below.



Comment at: lib/Format/UnwrappedLineParser.cpp:865
   case tok::kw_default:
-nextToken();
-parseLabel();
-return;
+if (Style.Language != FormatStyle::LK_Java || !Line->MustBeDeclaration) {
+  nextToken();

Change the order here. I.e.

  if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration)
break;
  ...

I think then you almost don't even need the comment.



Comment at: lib/Format/UnwrappedLineParser.cpp:870
+}
+// 'default' can appear in a Java 8 declaration. Parse it as such.
+break;

Is there a test case that hits this codepath? IIUC, it would need to have a 
"default" of a declaration that's not at the start of the line.


https://reviews.llvm.org/D27377



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

r289571 - [Sema] Prefer SmallVector over `new`ed memory blocks. NFC.

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Tue Dec 13 13:22:56 2016
New Revision: 289571

URL: http://llvm.org/viewvc/llvm-project?rev=289571&view=rev
Log:
[Sema] Prefer SmallVector over `new`ed memory blocks. NFC.

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=289571&r1=289570&r2=289571&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue Dec 13 13:22:56 2016
@@ -12749,9 +12749,9 @@ Sema::BuildCallToObjectOfClassType(Scope
 
   // Build the full argument list for the method call (the implicit object
   // parameter is placed at the beginning of the list).
-  std::unique_ptr MethodArgs(new Expr *[Args.size() + 1]);
+  SmallVector MethodArgs(Args.size() + 1);
   MethodArgs[0] = Object.get();
-  std::copy(Args.begin(), Args.end(), &MethodArgs[1]);
+  std::copy(Args.begin(), Args.end(), MethodArgs.begin() + 1);
 
   // Once we've built TheCall, all of the expressions are properly
   // owned.
@@ -12760,10 +12760,8 @@ Sema::BuildCallToObjectOfClassType(Scope
   ResultTy = ResultTy.getNonLValueExprType(Context);
 
   CXXOperatorCallExpr *TheCall = new (Context)
-  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(),
-  llvm::makeArrayRef(MethodArgs.get(), Args.size() + 
1),
-  ResultTy, VK, RParenLoc, false);
-  MethodArgs.reset();
+  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
+  VK, RParenLoc, false);
 
   if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
 return true;


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

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks!


https://reviews.llvm.org/D22296



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

Re: r289413 - Add two new AST nodes to represent initialization of an array in terms of

[Reid Kleckner via cfe-commits]

Aha, you have activated the win32 stack alignment trap card. By adding a
uint64_t to EvalInfo, you have increased its alignment to 8. Unfortunately,
MSVC doesn't actually align stack objects to more than 4 unless you really
ask it to with __declspec(align). Normally this stuff flies under the
radar, but PointerUnion makes assertions about alignment. A possible fix:

diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index 61bb2b9..e18caff 100644
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -433,7 +433,7 @@ namespace {
   /// rules.  For example, the RHS of (0 && foo()) is not evaluated.  We
can
   /// evaluate the expression regardless of what the RHS is, but C only
allows
   /// certain things in certain situations.
-  struct EvalInfo {
+  struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo {
 ASTContext &Ctx;

 /// EvalStatus - Contains information about the evaluation.

This has the downside that it will emit more stack realignment prologues
for MSVC x86 builds. I know Sony cares about MSVC-built clang performance,
but I don't know if they ship 32-bit or 64-bit binaries.

On Tue, Dec 13, 2016 at 11:43 AM, Reid Kleckner  wrote:

> It's probably this change:
>
> $ "C:/src/llvm/build_x86/./bin/clang.EXE" "-cc1" "-internal-isystem"
> "C:\src\llvm\build_x86\bin\..\lib\clang\4.0.0\include" "-nostdsysteminc"
> "-fsyntax-only" "-Wno-everything" "-Wobjc-literal-compare" "-Dnil=(id)0"
> "-verify" "C:\src\llvm\tools\clang\test\SemaObjC\objc-literal-
> comparison.m"
> # command stderr:
> Assertion failed: (PtrWord & ~PointerBitMask) == 0 && "Pointer is not
> sufficiently aligned", file C:\src\llvm\include\llvm/ADT/PointerIntPair.h,
> line 160
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25263: [Driver] Allow setting the default linker during build

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 81276.
phosek marked 3 inline comments as done.

Repository:
  rL LLVM

https://reviews.llvm.org/D25263

Files:
  CMakeLists.txt
  include/clang/Config/config.h.cmake
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h

Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -961,6 +961,10 @@
  : RuntimeLibType::RLT_CompilerRT;
   }
 
+  const char *getDefaultLinker() const override {
+return "lld";
+  }
+
 private:
   Multilib SelectedMultilib;
   std::string LibSuffix;
@@ -1090,6 +1094,10 @@
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
+  const char *getDefaultLinker() const override {
+return "lld";
+  }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
@@ -1289,6 +1297,10 @@
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
+  const char *getDefaultLinker() const override {
+return "lld";
+  }
+
   Tool *buildLinker() const override;
 };
 
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -3059,9 +3059,6 @@
   LibSuffix = tools::mips::getMipsABILibSuffix(Args, Triple);
   getFilePaths().clear();
   getFilePaths().push_back(computeSysRoot() + "/usr/lib" + LibSuffix);
-
-  // Use LLD by default.
-  DefaultLinker = "lld";
 }
 
 void MipsLLVMToolChain::AddClangSystemIncludeArgs(
@@ -4749,9 +4746,6 @@
 
   getFilePaths().push_back(D.SysRoot + "/lib");
   getFilePaths().push_back(D.ResourceDir + "/lib/fuchsia");
-
-  // Use LLD by default.
-  DefaultLinker = "lld";
 }
 
 Tool *Fuchsia::buildAssembler() const {
@@ -5173,9 +5167,6 @@
   assert(Triple.isArch32Bit() != Triple.isArch64Bit());
   getFilePaths().push_back(
   getDriver().SysRoot + "/lib" + (Triple.isArch32Bit() ? "32" : "64"));
-
-  // Use LLD by default.
-  DefaultLinker = "lld";
 }
 
 bool WebAssembly::IsMathErrnoDefault() const { return false; }
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -351,33 +351,31 @@
 }
 
 std::string ToolChain::GetLinkerPath() const {
-  if (Arg *A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef UseLinker = A->getValue();
-
-if (llvm::sys::path::is_absolute(UseLinker)) {
-  // If we're passed -fuse-ld= with what looks like an absolute path,
-  // don't attempt to second-guess that.
-  if (llvm::sys::fs::exists(UseLinker))
-return UseLinker;
-} else {
-  // If we're passed -fuse-ld= with no argument, or with the argument ld,
-  // then use whatever the default system linker is.
-  if (UseLinker.empty() || UseLinker == "ld")
-return GetProgramPath("ld");
-
-  llvm::SmallString<8> LinkerName("ld.");
-  LinkerName.append(UseLinker);
-
-  std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-  if (llvm::sys::fs::exists(LinkerPath))
-return LinkerPath;
-}
+  const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
+
+  if (llvm::sys::path::is_absolute(UseLinker)) {
+// If we're passed what looks like an absolute path, don't attempt to
+// second-guess that.
+if (llvm::sys::fs::exists(UseLinker))
+  return UseLinker;
+  } else if (UseLinker.empty() || UseLinker == "ld") {
+// If we're passed -fuse-ld= with no argument, or with the argument ld,
+// then use whatever the default system linker is.
+return GetProgramPath(getDefaultLinker());
+  } else {
+llvm::SmallString<8> LinkerName("ld.");
+LinkerName.append(UseLinker);
+
+std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
+if (llvm::sys::fs::exists(LinkerPath))
+  return LinkerPath;
+  }
 
+  if (A)
 getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args);
-return "";
-  }
 
-  return GetProgramPath(DefaultLinker);
+  return GetProgramPath(getDefaultLinker());
 }
 
 types::ID ToolChain::LookupTypeForExtension(StringRef Ext) const {
Index: include/clang/Driver/ToolChain.h
===
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -105,7 +105,6 @@
 
 protected:
   MultilibSet Multilibs;
-  const char *DefaultLinker = "ld";
 
   ToolChain(const Driver &D, const llvm::Triple &T,
 const llvm::opt::ArgList &Args);
@@ -272,6 +271,11 @@
 return 0;
   }
 
+  /// GetDefaultLinker - Get the default linker to use.
+  virtual const char *getDefaultLinker() const {
+return "

r289575 - Align EvalInfo in ExprConstant to avoid PointerUnion assertions

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Dec 13 13:48:32 2016
New Revision: 289575

URL: http://llvm.org/viewvc/llvm-project?rev=289575&view=rev
Log:
Align EvalInfo in ExprConstant to avoid PointerUnion assertions

32-bit MSVC doesn't provide more than 4 byte stack alignment by default.
This conflicts with PointerUnion's attempt to make assertions about
alignment. This fixes the problem by explicitly asking the compiler for
8 byte alignment.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=289575&r1=289574&r2=289575&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Dec 13 13:48:32 2016
@@ -433,7 +433,7 @@ namespace {
   /// rules.  For example, the RHS of (0 && foo()) is not evaluated.  We can
   /// evaluate the expression regardless of what the RHS is, but C only allows
   /// certain things in certain situations.
-  struct EvalInfo {
+  struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo {
 ASTContext &Ctx;
 
 /// EvalStatus - Contains information about the evaluation.


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

Re: r289544 - Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.

[Tom Stellard via cfe-commits]

On Tue, Dec 13, 2016 at 04:22:50PM -, Neil Hickey via cfe-commits wrote:
> Author: neil.hickey
> Date: Tue Dec 13 10:22:50 2016
> New Revision: 289544
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=289544&view=rev
> Log:
> Improve handling of floating point literals in OpenCL to only use double 
> precision if the target supports fp64.
> 
> This change makes sure single-precision floating point types are used if the 
> cl_fp64 extension is not supported by the target.
> 
> Also removed the check to see whether the OpenCL version is >= 1.2, as this 
> has
> been incorporated into the extension setting code.
> 
> Differential Revision: https://reviews.llvm.org/D24235
> 
> 

This patch causes a crash when compiling libclc.  You can reproduce by
compiling this function:

int isnan(float x){ return __builtin_isnan(x); }

As I recall there is some weirdness with how clang does type
deduction for these builtins, but the compiler shouldn't be crashing.

-Tom


> Modified:
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaType.cpp
> cfe/trunk/test/CodeGenOpenCL/fpmath.cl
> cfe/trunk/test/SemaOpenCL/extensions.cl
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 13 10:22:50 2016
> @@ -3748,12 +3748,13 @@ bool Sema::SemaBuiltinFPClassification(C
>  diag::err_typecheck_call_invalid_unary_fp)
><< OrigArg->getType() << OrigArg->getSourceRange();
>  
> -  // If this is an implicit conversion from float -> double, remove it.
> +  // If this is an implicit conversion from float -> float or double, remove 
> it.
>if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
>  Expr *CastArg = Cast->getSubExpr();
>  if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
> -  assert(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) &&
> - "promotion from float to double is the only expected cast 
> here");
> +assert((Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) 
> ||
> +Cast->getType()->isSpecificBuiltinType(BuiltinType::Float)) 
> &&
> + "promotion from float to either float or double is the only 
> expected cast here");
>Cast->setSubExpr(nullptr);
>TheCall->setArg(NumArgs-1, CastArg);
>  }
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Dec 13 10:22:50 2016
> @@ -817,8 +817,16 @@ ExprResult Sema::DefaultArgumentPromotio
>// double.
>const BuiltinType *BTy = Ty->getAs();
>if (BTy && (BTy->getKind() == BuiltinType::Half ||
> -  BTy->getKind() == BuiltinType::Float))
> -E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
> +  BTy->getKind() == BuiltinType::Float)) {
> +if (getLangOpts().OpenCL &&
> +!(getOpenCLOptions().cl_khr_fp64)) {
> +if (BTy->getKind() == BuiltinType::Half) {
> +E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
> +}
> +} else {
> +  E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
> +}
> +  }
>  
>// C++ performs lvalue-to-rvalue conversion as a default argument
>// promotion, even on class types, but note:
> @@ -3397,10 +3405,13 @@ ExprResult Sema::ActOnNumericConstant(co
>  
>  if (Ty == Context.DoubleTy) {
>if (getLangOpts().SinglePrecisionConstants) {
> -Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
> +const BuiltinType *BTy = Ty->getAs();
> +if (BTy->getKind() != BuiltinType::Float) {
> +  Res = ImpCastExprToType(Res, Context.FloatTy, 
> CK_FloatingCast).get();
> +}
>} else if (getLangOpts().OpenCL &&
> - !((getLangOpts().OpenCLVersion >= 120) ||
> -   getOpenCLOptions().cl_khr_fp64)) {
> + !(getOpenCLOptions().cl_khr_fp64)) {
> +// Impose single-precision float type when cl_khr_fp64 is not 
> enabled.
>  Diag(Tok.getLocation(), diag::warn_double_const_requires_fp64);
>  Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
>}
> 
> Modified: cfe/trunk/lib/Sema/SemaType.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==
> --- cfe/trunk/l

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

[Luis Héctor Chávez via Phabricator via cfe-commits]

lhchavez updated this revision to Diff 81280.
lhchavez added a comment.

Addressed feedback


https://reviews.llvm.org/D27377

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJava.cpp


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -515,5 +515,22 @@
"  void f() {}"));
 }
 
+TEST_F(FormatTestJava, UnderstandsDefaultModifier) {
+  verifyFormat("class SomeClass {\n"
+   "  default void f() {}\n"
+   "  public default void g() {}\n"
+   "  default public void h() {}\n"
+   "  int i() {\n"
+   "switch (0) {\n"
+   "  case 0:\n"
+   "break;\n"
+   "  default:\n"
+   "break;\n"
+   "}\n"
+   "  }\n"
+   "}",
+   getGoogleStyle(FormatStyle::LK_Java));
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -294,6 +294,11 @@
   addUnwrappedLine();
   break;
 case tok::kw_default:
+  if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration) {
+parseStructuralElement();
+break;
+  }
+  // fallthrough
 case tok::kw_case:
   if (!SwitchLabelEncountered &&
   (Style.IndentCaseLabels || (Line->InPPDirective && Line->Level == 
1)))
@@ -853,6 +858,8 @@
 parseSwitch();
 return;
   case tok::kw_default:
+if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration)
+  break;
 nextToken();
 parseLabel();
 return;


Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -515,5 +515,22 @@
"  void f() {}"));
 }
 
+TEST_F(FormatTestJava, UnderstandsDefaultModifier) {
+  verifyFormat("class SomeClass {\n"
+   "  default void f() {}\n"
+   "  public default void g() {}\n"
+   "  default public void h() {}\n"
+   "  int i() {\n"
+   "switch (0) {\n"
+   "  case 0:\n"
+   "break;\n"
+   "  default:\n"
+   "break;\n"
+   "}\n"
+   "  }\n"
+   "}",
+   getGoogleStyle(FormatStyle::LK_Java));
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -294,6 +294,11 @@
   addUnwrappedLine();
   break;
 case tok::kw_default:
+  if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration) {
+parseStructuralElement();
+break;
+  }
+  // fallthrough
 case tok::kw_case:
   if (!SwitchLabelEncountered &&
   (Style.IndentCaseLabels || (Line->InPPDirective && Line->Level == 1)))
@@ -853,6 +858,8 @@
 parseSwitch();
 return;
   case tok::kw_default:
+if (Style.Language == FormatStyle::LK_Java && Line->MustBeDeclaration)
+  break;
 nextToken();
 parseLabel();
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

[Luis Héctor Chávez via Phabricator via cfe-commits]

lhchavez marked 3 inline comments as done.
lhchavez added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:870
+}
+// 'default' can appear in a Java 8 declaration. Parse it as such.
+break;

djasper wrote:
> Is there a test case that hits this codepath? IIUC, it would need to have a 
> "default" of a declaration that's not at the start of the line.
Added two more test cases.


https://reviews.llvm.org/D27377



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

[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

Do the fixits work when the object is a pointer to a string?
`pstr1->compare(str2) == 0`
Does the warning work when the argument is a pointer to a string?
`str1.compare(*pstr2) == 0`
Does the warning work when the argument is the result of a function call?
Does the warning work when the object is the result of a function call?


https://reviews.llvm.org/D27210



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

[PATCH] D26167: [Clang-tidy] check for malloc, realloc and free calls

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

nit: All of the new files had UTF-8 BOMs, which I don't think we usually add.




Comment at: test/clang-tidy/cppcoreguidelines-no-malloc.cpp:3
+
+using size_t = unsigned long;
+

This test did not pass on 32-bit Windows. I will change it to:
  using size_t = __SIZE_TYPE__;


Repository:
  rL LLVM

https://reviews.llvm.org/D26167



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

[clang-tools-extra] r289580 - Fix size_t typdef in new cppcoreguidelines-no-malloc.cpp test

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Dec 13 14:25:47 2016
New Revision: 289580

URL: http://llvm.org/viewvc/llvm-project?rev=289580&view=rev
Log:
Fix size_t typdef in new cppcoreguidelines-no-malloc.cpp test

Modified:
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp?rev=289580&r1=289579&r2=289580&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-no-malloc.cpp Tue 
Dec 13 14:25:47 2016
@@ -1,6 +1,6 @@
 // RUN: %check_clang_tidy %s cppcoreguidelines-no-malloc %t
 
-using size_t = unsigned long;
+using size_t = __SIZE_TYPE__;
 
 void *malloc(size_t size);
 void *calloc(size_t num, size_t size);


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

[clang-tools-extra] r289581 - [powerpc] deactivate readability-identifier-naming.cpp test on powerpc64le

[Bill Seurer via cfe-commits]

Author: seurer
Date: Tue Dec 13 14:26:35 2016
New Revision: 289581

URL: http://llvm.org/viewvc/llvm-project?rev=289581&view=rev
Log:
[powerpc] deactivate readability-identifier-naming.cpp test on powerpc64le

The test case clang-tidy/readability-identifier-naming.cpp segfaults on
powerpc64 little endian (starting with r288563) when a bootstrap build/test
is done.  To get the buildbot running again deactivate the test.
When the issue is resolved reactivate it.


Modified:
clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp?rev=289581&r1=289580&r2=289581&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp 
Tue Dec 13 14:26:35 2016
@@ -1,3 +1,6 @@
+// Remove UNSUPPORTED for powerpc64le when the problem introduced by
+// r288563 is resolved.
+// UNSUPPORTED: powerpc64le
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.AbstractClassCase, value: 
CamelCase}, \


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

[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 81284.
ahatanak added a comment.

Address review comment. Use llvm::is_contained instead of std::find.


https://reviews.llvm.org/D27680

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/lifetime2.c

Index: test/CodeGen/lifetime2.c
===
--- test/CodeGen/lifetime2.c
+++ test/CodeGen/lifetime2.c
@@ -19,11 +19,11 @@
 
 // CHECK-LABEL: @no_goto_bypass
 void no_goto_bypass() {
+  // O2: @llvm.lifetime.start(i64 5
   // O2: @llvm.lifetime.start(i64 1
   char x;
 l1:
   bar(&x, 1);
-  // O2: @llvm.lifetime.start(i64 5
   // O2: @llvm.lifetime.end(i64 5
   char y[5];
   bar(y, 5);
@@ -89,3 +89,27 @@
 L:
   bar(&x, 1);
 }
+
+// O2: %[[ALLOCA:[a-z0-9]+]] = alloca i32, align 4
+// O2: %[[BITCAST:[0-9]+]] = bitcast i32* %[[ALLOCA]] to i8*
+// O2: call void @llvm.lifetime.start(i64 4, i8* nonnull %[[BITCAST]])
+// O2: [[DESTLABEL:[a-z0-9]+]]:
+// O2: br {{.*}} label %[[DESTLABEL]]
+
+extern void foo2(int p);
+
+int move_lifetime_start(int a) {
+  int *p = 0;
+ label1:
+  if (p) {
+foo2(*p);
+return 0;
+  }
+
+  int i = 999;
+  if (a != 2) {
+p = &i;
+goto label1;
+  }
+  return -1;
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -322,14 +322,20 @@
   class CallLifetimeEnd final : public EHScopeStack::Cleanup {
 llvm::Value *Addr;
 llvm::Value *Size;
+llvm::CallInst *LifetimeStart;
 
   public:
-CallLifetimeEnd(Address addr, llvm::Value *size)
-: Addr(addr.getPointer()), Size(size) {}
+CallLifetimeEnd(Address addr, llvm::Value *size,
+llvm::CallInst *lifetimeStart = nullptr)
+: Addr(addr.getPointer()), Size(size), LifetimeStart(lifetimeStart) {}
 
 void Emit(CodeGenFunction &CGF, Flags flags) override {
   CGF.EmitLifetimeEnd(Size, Addr);
 }
+
+llvm::CallInst *&getLifetimeStart() {
+  return LifetimeStart;
+}
   };
 
   /// Header for data within LifetimeExtendedCleanupStack.
@@ -497,10 +503,10 @@
   /// \brief Enters a new scope for capturing cleanups, all of which
   /// will be executed once the scope is exited.
   class RunCleanupsScope {
-EHScopeStack::stable_iterator CleanupStackDepth;
 size_t LifetimeExtendedCleanupStackSize;
 bool OldDidCallStackSave;
   protected:
+EHScopeStack::stable_iterator CleanupStackDepth;
 bool PerformCleanup;
   private:
 
@@ -552,6 +558,7 @@
 SourceRange Range;
 SmallVector Labels;
 LexicalScope *ParentScope;
+llvm::BasicBlock *BB;
 
 LexicalScope(const LexicalScope &) = delete;
 void operator=(const LexicalScope &) = delete;
@@ -563,13 +570,27 @@
   CGF.CurLexicalScope = this;
   if (CGDebugInfo *DI = CGF.getDebugInfo())
 DI->EmitLexicalBlockStart(CGF.Builder, Range.getBegin());
+  CGF.EnsureInsertPoint();
+  BB = CGF.Builder.GetInsertBlock();
 }
 
 void addLabel(const LabelDecl *label) {
   assert(PerformCleanup && "adding label to dead scope?");
   Labels.push_back(label);
 }
 
+EHScopeStack::stable_iterator getCleanupStackDepth() const {
+  return CleanupStackDepth;
+}
+
+llvm::BasicBlock *getBasicBlock() const {
+  return BB;
+}
+
+LexicalScope *getParentScope() const {
+  return ParentScope;
+}
+
 /// \brief Exit this cleanup scope, emitting any accumulated
 /// cleanups.
 ~LexicalScope() {
@@ -594,6 +615,10 @@
 rescopeLabels();
 }
 
+bool hasLabel(const LabelDecl *LD) const {
+  return llvm::is_contained(Labels, LD);
+}
+
 void rescopeLabels();
   };
 
@@ -2124,7 +2149,7 @@
   void EmitCXXTemporary(const CXXTemporary *Temporary, QualType TempType,
 Address Ptr);
 
-  llvm::Value *EmitLifetimeStart(uint64_t Size, llvm::Value *Addr);
+  llvm::CallInst *EmitLifetimeStart(uint64_t Size, llvm::Value *Addr);
   void EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr);
 
   llvm::Value *EmitCXXNewExpr(const CXXNewExpr *E);
@@ -2253,27 +2278,30 @@
 bool IsConstantAggregate;
 
 /// Non-null if we should use lifetime annotations.
-llvm::Value *SizeForLifetimeMarkers;
+llvm::CallInst *LifetimeStart;
 
 struct Invalid {};
 AutoVarEmission(Invalid) : Variable(nullptr), Addr(Address::invalid()) {}
 
 AutoVarEmission(const VarDecl &variable)
   : Variable(&variable), Addr(Address::invalid()), NRVOFlag(nullptr),
-IsByRef(false), IsConstantAggregate(false),
-SizeForLifetimeMarkers(nullptr) {}
+IsByRef(false), IsConstantAggregate(false), LifetimeStart(nullptr) {}
 
 bool wasEmittedAsGlobal() const { return !Addr.isValid(); }
 
   public:
 static AutoVarEmission invalid() { return AutoVarEmission(Invalid()); }
 
 bool useLifetimeMarkers() con

[PATCH] D27726: [analyzer] Refer to macro names in diagnostics for macros representing a literal

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna created this revision.
zaks.anna added reviewers: dergachev.a, dcoughlin.
zaks.anna added a subscriber: cfe-commits.

When a macro expending to a literal is used in a comparison, use the macro name 
in the diagnostic rather than the literal. This improves readability of path 
notes.

Added tests for various macro literals that could occur. Only BOOl, Int, and 
NULL tests have changed behavior with this patch.


https://reviews.llvm.org/D27726

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/Inputs/system-header-simulator-objc.h
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/diagnostics/macros.m

Index: test/Analysis/diagnostics/macros.m
===
--- /dev/null
+++ test/Analysis/diagnostics/macros.m
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx -fblocks -analyzer-output=text -verify %s
+
+#include "../Inputs/system-header-simulator-objc.h"
+
+@interface NSDictionary : NSObject
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
+@end
+@interface NSMutableDictionary : NSDictionary
+- (void)setObject:(id)anObject forKey:(id )aKey;
+@end
+
+void testBOOLMacro(BOOL b) {
+  if (b == YES) { // expected-note {{Assuming 'b' is equal to YES}}
+  // expected-note@-1 {{Taking true branch}}
+char *p = NULL;// expected-note {{'p' initialized to a null pointer value}}
+*p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+ // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+void testNilMacro(NSMutableDictionary *d, NSObject *o) {
+  if (o == nil) // expected-note {{Assuming 'o' is equal to nil}}
+// expected-note@-1 {{Taking true branch}}
+[d setObject:o forKey:[o description]]; // expected-warning {{Key argument to 'setObject:forKey:' cannot be nil}}
+// expected-note@-1 {{'description' not called because the receiver is nil}}
+// expected-note@-2 {{Key argument to 'setObject:forKey:' cannot be nil}}
+
+  return;
+}
Index: test/Analysis/diagnostics/macros.cpp
===
--- /dev/null
+++ test/Analysis/diagnostics/macros.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+#include "../Inputs/system-header-simulator-cxx.h"
+
+void testIntMacro(unsigned int i) {
+  if (i == UINT32_MAX) { // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
+ // expected-note@-1 {{Taking true branch}}
+char *p = NULL; // expected-note {{'p' initialized to a null pointer value}}
+*p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+ // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+void testNULLMacro(int *p) {
+  if (p == NULL) { // expected-note {{Assuming 'p' is equal to NULL}}
+   // expected-note@-1 {{Taking true branch}}
+*p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+ // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+void testnullptrMacro(int *p) {
+  if (p == nullptr) { // expected-note {{Assuming pointer value is null}}
+  // expected-note@-1 {{Taking true branch}}
+*p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+ // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+// There are no path notes on the comparison to float types.
+void testDoubleMacro(double d) {
+  if (d == DBL_MAX) { // expected-note {{Taking true branch}}
+
+char *p = NULL; // expected-note {{'p' initialized to a null pointer value}}
+*p = 7; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+void testboolMacro(bool b, int *p) {
+  p = nullptr;  // expected-note {{Null pointer value stored to 'p'}}
+  if (b == false) { // expected-note {{Assuming the condition is true}}
+// expected-note@-1 {{Taking true branch}}
+*p = 7; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
\ No newline at end of file
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inpu

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.
Closed by commit rL289584: CodeGen: New vtable group representation: struct of 
vtable arrays. (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D22296?vs=81264&id=81288#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22296

Files:
  cfe/trunk/include/clang/AST/VTableBuilder.h
  cfe/trunk/include/clang/Basic/LLVM.h
  cfe/trunk/lib/AST/VTableBuilder.cpp
  cfe/trunk/lib/CodeGen/CGCXX.cpp
  cfe/trunk/lib/CodeGen/CGVTT.cpp
  cfe/trunk/lib/CodeGen/CGVTables.cpp
  cfe/trunk/lib/CodeGen/CGVTables.h
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
  cfe/trunk/test/CodeGenCXX/PR26569.cpp
  cfe/trunk/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
  cfe/trunk/test/CodeGenCXX/apple-kext-indirect-call.cpp
  cfe/trunk/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  cfe/trunk/test/CodeGenCXX/cfi-cross-dso.cpp
  cfe/trunk/test/CodeGenCXX/const-init-cxx11.cpp
  cfe/trunk/test/CodeGenCXX/constructor-init.cpp
  cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
  cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis.cpp
  cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
  cfe/trunk/test/CodeGenCXX/dllexport.cpp
  cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp
  cfe/trunk/test/CodeGenCXX/dllimport.cpp
  cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp
  cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-interface.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-no-rtti-data.cpp
  cfe/trunk/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
  cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp
  cfe/trunk/test/CodeGenCXX/vtable-align.cpp
  cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp
  cfe/trunk/test/CodeGenCXX/vtable-pointer-initialization.cpp
  cfe/trunk/test/CodeGenCXX/vtt-layout.cpp

Index: cfe/trunk/include/clang/AST/VTableBuilder.h
===
--- cfe/trunk/include/clang/AST/VTableBuilder.h
+++ cfe/trunk/include/clang/AST/VTableBuilder.h
@@ -218,52 +218,77 @@
 class VTableLayout {
 public:
   typedef std::pair VTableThunkTy;
-  typedef llvm::DenseMap AddressPointsMapTy;
+  struct AddressPointLocation {
+unsigned VTableIndex, AddressPointIndex;
+  };
+  typedef llvm::DenseMap
+  AddressPointsMapTy;
 
 private:
-  uint64_t NumVTableComponents;
-  std::unique_ptr VTableComponents;
+  // Stores the component indices of the first component of each virtual table in
+  // the virtual table group. To save a little memory in the common case where
+  // the vtable group contains a single vtable, an empty vector here represents
+  // the vector {0}.
+  OwningArrayRef VTableIndices;
+
+  OwningArrayRef VTableComponents;
 
   /// \brief Contains thunks needed by vtables, sorted by indices.
-  uint64_t NumVTableThunks;
-  std::unique_ptr VTableThunks;
+  OwningArrayRef VTableThunks;
 
   /// \brief Address points for all vtables.
   AddressPointsMapTy AddressPoints;
 
-  bool IsMicrosoftABI;
-
 public:
-  VTableLayout(uint64_t NumVTableComponents,
-   const VTableComponent *VTableComponents,
-   uint64_t NumVTableThunks,
-   const VTableThunkTy *VTableThunks,
-   const AddressPointsMapTy &AddressPoints,
-   bool IsMicrosoftABI);
+  VTableLayout(ArrayRef VTableIndices,
+   ArrayRef VTableComponents,
+   ArrayRef VTableThunks,
+   const AddressPointsMapTy &AddressPoints);
   ~VTableLayout();
 
   ArrayRef vtable_components() const {
-return {VTableComponents.get(), size_t(NumVTableComponents)};
+return VTableComponents;
   }
 
   ArrayRef vtable_thunks() const {
-return {VTableThunks.get(), size_t(NumVTableThunks)};
+return VTableThunks;
   }
 
-  uint64_t getAddressPoint(BaseSubobject Base) const {
-assert(AddressPoints.count(Base) &&
-   "Did not find address point!");
-
-uint64_t AddressPoint = AddressPoints.lookup(Base);
-assert(AddressPoint != 0 || IsMicrosoftABI);
-(void)IsMicrosoftABI;
-
-return AddressPoint;
+  AddressPointLocation getAddressPoint(BaseSubobject Base) const {
+assert(AddressPoints.count(Base) && "Did not find address point!");
+return AddressPoints.find(Base)->second;
   }
 
   con