Re: [PATCH] D24245: [ARM] ARM-specific attributes should be accepted for big-endian

[Oliver Stannard via cfe-commits]

olista01 added a comment.

Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D24245



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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

In https://reviews.llvm.org/D24349#538102, @aaron.ballman wrote:

> In https://reviews.llvm.org/D24349#538098, @alexfh wrote:
>
> > Thank you for the patch!
> >
> > In https://reviews.llvm.org/D24349#537500, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
> > >
> > > > Probably check should have options to extend list of containers and 
> > > > also to assume all classes with integer type size() const and bool 
> > > > empty() const as containers. It may be not trivial to find out all 
> > > > custom containers and last option will be helpful to assemble such list.
> > >
> > >
> > > I think this should actually have two options: one is a list of 
> > > default-checked containers (default is the list of 
> > >  STL containers we previously had), and the other is a Boolean option for 
> > > guessing at what might be a 
> > >  container based on function signature (default is true). This gives 
> > > people a way to silence the diagnostic 
> > >  while still being useful.
> >
> >
> > While I understand your concerns, I would suggest to wait for actual bug 
> > reports before introducing more options. The duck typing approach worked 
> > really well for readability-redundant-smartptr-get and I expect it to work 
> > equally well here. The constraints checked here are pretty strict and 
> > should make the check safe.
>
>
> That means there is no way to silence this check on false positives aside 
> from disabling it entirely, which is not a particularly good user experience. 
> However, I do agree that if we constrain the heuristic appropriately, the 
> number of false positives should be low.


Just checked: readability-container-size-empty with the patch finds a few 
thousand new warnings related to about 400 unique container names in our code. 
A representative sample shows no false positives and we've found just a single 
false negative (that triggers the check without the patch) - a container's 
`size()` method was not `const`. Which makes me pretty sure that:

1. The new algorithm is much more consistent and useful than the whitelist 
approach (gathering and maintaining a whitelist of a few hundred names is also 
not fun at all).
2. Given the size of our code and the number of third-party libraries, 
possibility of false positives is extremely low.
3. If there are reports of false positives, we could go with a blacklist 
approach instead, but I don't think this will ever be needed.



Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+  const auto validContainer = cxxRecordDecl(isSameOrDerivedFrom(
+  namedDecl(has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+  hasName("size"), returns(isInteger()),

I don't think we can come up with a proper solution until we see a real world 
example. If we want to find one faster, we can allow all character types and 
wait for reports of false positives ;)


Comment at: docs/clang-tidy/checks/readability-container-size-empty.rst:18
@@ +17,3 @@
+
+Check issues warning if a container has `size()` and `empty()` methods matching
+following signatures:

`The check`


Comment at: docs/clang-tidy/checks/readability-container-size-empty.rst:18
@@ +17,3 @@
+
+Check issues warning if a container has `size()` and `empty()` methods matching
+following signatures:

alexfh wrote:
> `The check`
Double backquotes should be used.


Comment at: docs/clang-tidy/checks/readability-container-size-empty.rst:21
@@ +20,3 @@
+
+code-block::c++
+

nit: space before `c++`.


Comment at: docs/clang-tidy/checks/readability-container-size-empty.rst:23
@@ +22,3 @@
+
+  size_type size() const;
+  bool empty() const;

Explain the constraints `size_type` should satisfy.


https://reviews.llvm.org/D24349



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

Re: [PATCH] D24238: StaticAnalyzer CastToStruct : No memory corruption when casting array to struct

[Daniel Marjamäki via cfe-commits]

danielmarjamaki abandoned this revision.
danielmarjamaki added a comment.

hmm.. I don't actually care much about this specific check at the moment.

I saw other false positives (unreachable code) and thought that this check made 
the analyzer think there was corrupted memory.

Now I can't reproduce my other false positives.

I'll close this for now. Unless I get those other FP again I don't care about 
this.


https://reviews.llvm.org/D24238



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

Re: [PATCH] D23842: [CFG] Add iterator_ranges to CFG and CFGBlock.

[Martin Böhme via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rL281200: [CFG] Add iterator_ranges to CFG and CFGBlock. 
(authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D23842?vs=69129=70981#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23842

Files:
  cfe/trunk/include/clang/Analysis/CFG.h

Index: cfe/trunk/include/clang/Analysis/CFG.h
===
--- cfe/trunk/include/clang/Analysis/CFG.h
+++ cfe/trunk/include/clang/Analysis/CFG.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
@@ -522,11 +523,15 @@
   typedef AdjacentBlocks::const_iterator  const_pred_iterator;
   typedef AdjacentBlocks::reverse_iterator  pred_reverse_iterator;
   typedef AdjacentBlocks::const_reverse_iterator  const_pred_reverse_iterator;
+  typedef llvm::iterator_range  pred_range;
+  typedef llvm::iterator_range  pred_const_range;
 
   typedef AdjacentBlocks::iterator  succ_iterator;
   typedef AdjacentBlocks::const_iterator  const_succ_iterator;
   typedef AdjacentBlocks::reverse_iterator  succ_reverse_iterator;
   typedef AdjacentBlocks::const_reverse_iterator  const_succ_reverse_iterator;
+  typedef llvm::iterator_range  succ_range;
+  typedef llvm::iterator_range  succ_const_range;
 
   pred_iteratorpred_begin(){ return Preds.begin();   }
   pred_iteratorpred_end()  { return Preds.end(); }
@@ -538,6 +543,13 @@
   const_pred_reverse_iterator  pred_rbegin() const { return Preds.rbegin();  }
   const_pred_reverse_iterator  pred_rend()   const { return Preds.rend();}
 
+  pred_range   preds() {
+return pred_range(pred_begin(), pred_end());
+  }
+  pred_const_range preds() const {
+return pred_const_range(pred_begin(), pred_end());
+  }
+
   succ_iteratorsucc_begin(){ return Succs.begin();   }
   succ_iteratorsucc_end()  { return Succs.end(); }
   const_succ_iterator  succ_begin()  const { return Succs.begin();   }
@@ -548,6 +560,13 @@
   const_succ_reverse_iterator  succ_rbegin() const { return Succs.rbegin();  }
   const_succ_reverse_iterator  succ_rend()   const { return Succs.rend();}
 
+  succ_range   succs() {
+return succ_range(succ_begin(), succ_end());
+  }
+  succ_const_range succs() const {
+return succ_const_range(succ_begin(), succ_end());
+  }
+
   unsigned succ_size()   const { return Succs.size();}
   bool succ_empty()  const { return Succs.empty();   }
 
@@ -840,6 +859,7 @@
 
   typedef llvm::DenseMap::const_iterator
 synthetic_stmt_iterator;
+  typedef llvm::iterator_range synthetic_stmt_range;
 
   /// Iterates over synthetic DeclStmts in the CFG.
   ///
@@ -855,6 +875,11 @@
 return SyntheticDeclStmts.end();
   }
 
+  /// \sa synthetic_stmt_begin
+  synthetic_stmt_range synthetic_stmts() const {
+return synthetic_stmt_range(synthetic_stmt_begin(), synthetic_stmt_end());
+  }
+
   
//======//
   // Member templates useful for various batch operations over CFGs.
   
//======//


Index: cfe/trunk/include/clang/Analysis/CFG.h
===
--- cfe/trunk/include/clang/Analysis/CFG.h
+++ cfe/trunk/include/clang/Analysis/CFG.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
@@ -522,11 +523,15 @@
   typedef AdjacentBlocks::const_iterator  const_pred_iterator;
   typedef AdjacentBlocks::reverse_iterator  pred_reverse_iterator;
   typedef AdjacentBlocks::const_reverse_iterator  const_pred_reverse_iterator;
+  typedef llvm::iterator_range  pred_range;
+  typedef llvm::iterator_range  pred_const_range;
 
   typedef AdjacentBlocks::iterator  succ_iterator;
   typedef AdjacentBlocks::const_iterator  const_succ_iterator;
   typedef AdjacentBlocks::reverse_iterator  succ_reverse_iterator;
   typedef AdjacentBlocks::const_reverse_iterator  const_succ_reverse_iterator;
+  typedef llvm::iterator_range  succ_range;
+  typedef 

Re: [PATCH] D24380: [migrate-tool] Framework for a codebase-dependent migration tool.

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: migrate-tool/BuildManager.h:22
@@ +21,3 @@
+public:
+  virtual bool addHeaderOnlyLibrary(llvm::StringRef HeaderPath) = 0;
+

I'm not sure the header-only distinction makes sense.
I'd start with
virtual bool addLibrary(llvm::ArrayRef Sources) = 0;
or similar.

Also, needs more comments :) Especially what this would mean for different 
build systems.


Comment at: migrate-tool/BuildManager.h:27
@@ +26,3 @@
+
+  virtual std::string getTargetForFile(llvm::StringRef File) = 0;
+};

I think we'll want a more precise name, as this might mean different things 
(target that compiles File vs target that outputs File, etc).
What do you want this to do?


Comment at: migrate-tool/HeaderBuild.cpp:19
@@ +18,3 @@
+
+std::string joinStrings(const std::vector ) {
+std::ostringstream SS;

Can't we use join from ADT/StringExtras.h?


Comment at: migrate-tool/HeaderBuild.h:19
@@ +18,3 @@
+// This constructs a C++ header containing type aliases.
+class Header {
+public:

I'd probably call this HeaderGenerator or something.



Comment at: migrate-tool/HeaderBuild.h:29
@@ +28,3 @@
+
+  std::string generateContent() const;
+

This all needs more comments :)
I assume we'll also need to somehow give this a "style" at some point? Or 
alternatively create dumb data structures, and have a style interface that can 
create the actual source according to a style from the data we provide?


Comment at: migrate-tool/RefactorManager.h:24
@@ +23,3 @@
+// performs some refactoring operations.
+class RefactorManager {
+public:

bkramer wrote:
> Documentation for the methods? :)
I think "RefactoringManager" reads better; otherwise this sounds like you want 
to refactor the manager :)

That said, I think we'll need to split this - getAffectedFiles belongs in its 
own interface, and we might want to have a common interface with what Kirill 
and Benjamin are working on.


https://reviews.llvm.org/D24380



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

r281203 - clang-format: Make emacs integration work with narrowed buffers.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Mon Sep 12 05:02:46 2016
New Revision: 281203

URL: http://llvm.org/viewvc/llvm-project?rev=281203=rev
Log:
clang-format: Make emacs integration work with narrowed buffers.

Use (call-process region nil ...) instead of (point-min) so that the
call works in narrowed buffers.

Patch by Philipp Stephani, thank you!

Modified:
cfe/trunk/tools/clang-format/clang-format.el

Modified: cfe/trunk/tools/clang-format/clang-format.el
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.el?rev=281203=281202=281203=diff
==
--- cfe/trunk/tools/clang-format/clang-format.el (original)
+++ cfe/trunk/tools/clang-format/clang-format.el Mon Sep 12 05:02:46 2016
@@ -122,7 +122,7 @@ is no active region.  If no style is giv
 (let (status stderr operations)
   (setq status
 (call-process-region
- (point-min) (point-max) clang-format-executable
+ nil nil clang-format-executable
  nil `(,temp-buffer ,temp-file) nil
 
  "-output-replacements-xml"


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

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70989.
omtcyfz added a comment.

Messed up with the last diff; fix that + clang-format the check.


https://reviews.llvm.org/D24349

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  docs/clang-tidy/checks/readability-container-size-empty.rst
  test/clang-tidy/clang-cl-driver.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -24,9 +24,43 @@
 };
 }
 
-
 }
 
+template 
+class TemplatedContainer {
+public:
+  int size() const;
+  bool empty() const;
+};
+
+template 
+class PrivateEmpty {
+public:
+  int size() const;
+private:
+  bool empty() const;
+};
+
+struct BoolSize {
+  bool size() const;
+  bool empty() const;
+};
+
+struct EnumSize {
+  enum E { one };
+  enum E size() const;
+  bool empty() const;
+};
+
+class Container {
+public:
+  int size() const;
+  bool empty() const;
+};
+
+class Derived : public Container {
+};
+
 int main() {
   std::set intSet;
   std::string str;
@@ -39,6 +73,7 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
+  // CHECK-MESSAGES: :23:8: note: method 'set'::empty() defined here
   if (str.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
@@ -127,6 +162,116 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect4.empty()){{$}}
+
+  TemplatedContainer templated_container;
+  if (templated_container.size() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() != 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 == templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (0 != templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 < templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() < 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (1 > templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() >= 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (1 <= templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 1) // no warning
+;
+  if (1 < templated_container.size()) // no warning
+;
+  if (templated_container.size() <= 1) // no warning
+;
+  if (1 >= templated_container.size()) // no warning
+;
+  if (!templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+
+  if (templated_container.empty())
+;
+
+  // No warnings expected.
+  PrivateEmpty private_empty;
+  if (private_empty.size() == 0)
+;
+  if (private_empty.size() != 0)
+;
+  if (0 == private_empty.size())
+;
+  if (0 != private_empty.size())
+;
+  if (private_empty.size() > 0)
+;
+  if (0 < private_empty.size())
+;
+  if (private_empty.size() < 1)
+;
+  if (1 > private_empty.size())
+;
+  if (private_empty.size() >= 1)
+;
+  if (1 <= private_empty.size())
+;
+  if (private_empty.size() > 1)
+;
+  if (1 < private_empty.size())
+;
+  if (private_empty.size() <= 1)
+;
+ 

[PATCH] D24448: [atomics] New warning -Watomic-libcall when atomic operation expands to a library call

[Simon Dardis via cfe-commits]

sdardis created this revision.
sdardis added a subscriber: cfe-commits.
Herald added a reviewer: vkalintiris.
Herald added a subscriber: aemerson.

Targets typically support atomics that are word sized (e.g. 32 or 64 bit) or
half word sized (e.g. 32 bit on 64 bit systems). For larger sizes, some
targets can perform them directly (x86, ARM), other must expand them into
a library call. This patch adds the optional warning -Watomic-libcall which
warns when a atomic operation cannot be expanded inline and must use a
library call.

https://reviews.llvm.org/D24448

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/atomic-libcall.c

Index: test/Sema/atomic-libcall.c
===
--- /dev/null
+++ test/Sema/atomic-libcall.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=mips-mti-linux-gnu 
-Watomic-libcall
+
+// Test that larger than word size atomics are warned about.
+
+long long var;
+
+void foo(long long a) {
+  __sync_fetch_and_add(, 0, a); // expected-warning {{atomic builtin 
expands to library call}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3035,6 +3035,15 @@
 
   ASTContext& Context = this->getASTContext();
 
+  // Warn if the requested atomic will expand into a library call, as this can
+  // impact performance significantly if it requires kernel intervention or
+  // some other shared but hidden lock.
+  if (Context.getTypeSizeInChars(ValType) >
+  Context.toCharUnitsFromBits(
+  Context.getTargetInfo().getMaxAtomicInlineWidth())) {
+Diag(DRE->getLocStart(), diag::warn_atomic_builtin_expands_to_libcall);
+  }
+
   // Create a new DeclRefExpr to refer to the new decl.
   DeclRefExpr* NewDRE = DeclRefExpr::Create(
   Context,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5118,6 +5118,8 @@
   "_Atomic cannot be applied to "
   "%select{incomplete |array |function |reference |atomic |qualified |}0type "
   "%1 %select{||which is not trivially copyable}0">;
+def warn_atomic_builtin_expands_to_libcall : Warning<
+  "atomic builtin expands to library call">, InGroup, 
DefaultIgnore;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -859,6 +859,8 @@
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
 
+def AtomicLibcall : DiagGroup<"atomic-libcall">;
+
 // AddressSanitizer frontent instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;
 
@@ -880,4 +882,4 @@
 
 // A warning group for warnings about code that clang accepts when
 // compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
-def SpirCompat : DiagGroup<"spir-compat">;
\ No newline at end of file
+def SpirCompat : DiagGroup<"spir-compat">;


Index: test/Sema/atomic-libcall.c
===
--- /dev/null
+++ test/Sema/atomic-libcall.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=mips-mti-linux-gnu -Watomic-libcall
+
+// Test that larger than word size atomics are warned about.
+
+long long var;
+
+void foo(long long a) {
+  __sync_fetch_and_add(, 0, a); // expected-warning {{atomic builtin expands to library call}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3035,6 +3035,15 @@
 
   ASTContext& Context = this->getASTContext();
 
+  // Warn if the requested atomic will expand into a library call, as this can
+  // impact performance significantly if it requires kernel intervention or
+  // some other shared but hidden lock.
+  if (Context.getTypeSizeInChars(ValType) >
+  Context.toCharUnitsFromBits(
+  Context.getTargetInfo().getMaxAtomicInlineWidth())) {
+Diag(DRE->getLocStart(), diag::warn_atomic_builtin_expands_to_libcall);
+  }
+
   // Create a new DeclRefExpr to refer to the new decl.
   DeclRefExpr* NewDRE = DeclRefExpr::Create(
   Context,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5118,6 +5118,8 @@
   "_Atomic cannot be applied to "
   "%select{incomplete |array |function |reference |atomic |qualified |}0type "
   "%1 

Re: [PATCH] D24238: StaticAnalyzer CastToStruct : No memory corruption when casting array to struct

[Daniel Marjamäki via cfe-commits]

danielmarjamaki added a comment.

If the -fno-strict-aliasing would fix this warning then it would be OK.

If you are telling me that this CastToStruct check is about alignment or 
endianness then I think the message is highly misleading. We should rewrite the 
message.

In general, using char instead of short/int does not prevent 
alignment/endianness problems as far as I see. You can still have just as many 
such problems even though array is char.

I am not sure why they did not use char here. But on their platform, 
sizeof(char)==sizeof(short)==sizeof(int)==1. It does not matter much if a 
typedef uses char or int.


https://reviews.llvm.org/D24238



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

[Nikola Smiljanić via cfe-commits]

nikola updated this revision to Diff 70985.
nikola added a comment.

This should address Hans' comments, as for the code get I have no idea. I was 
hoping someone more knowledgeable would tell me if this made sense or not?


https://reviews.llvm.org/D24193

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/attr-naked.c

Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in 
naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not 
supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked 
function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM 
statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);


Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24349: [clang-tidy] Extend readability-container-size-empty to arbitrary class with size() and empty()

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 70986.
omtcyfz marked 4 inline comments as done.
omtcyfz added a comment.

Address another round of comments.


https://reviews.llvm.org/D24349

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  docs/clang-tidy/checks/readability-container-size-empty.rst
  test/clang-tidy/clang-cl-driver.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -24,9 +24,34 @@
 };
 }
 
-
 }
 
+template 
+class TemplatedContainer {
+public:
+  int size() const;
+  bool empty() const;
+};
+
+template 
+class PrivateEmpty {
+public:
+  int size() const;
+private:
+  bool empty() const;
+};
+
+struct BoolSize {
+  bool size() const;
+  bool empty() const;
+};
+
+struct EnumSize {
+  enum E { one };
+  enum E size() const;
+  bool empty() const;
+};
+
 int main() {
   std::set intSet;
   std::string str;
@@ -39,6 +64,7 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
+  // CHECK-MESSAGES: :23:8: note: method 'set'::empty() defined here
   if (str.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
@@ -127,6 +153,110 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect4.empty()){{$}}
+
+  TemplatedContainer templated_container;
+  if (templated_container.size() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() != 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 == templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (0 != templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (0 < templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() < 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (1 > templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size() >= 1)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (1 <= templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+  if (templated_container.size() > 1) // no warning
+;
+  if (1 < templated_container.size()) // no warning
+;
+  if (templated_container.size() <= 1) // no warning
+;
+  if (1 >= templated_container.size()) // no warning
+;
+  if (!templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
+  if (templated_container.size())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
+
+  if (templated_container.empty())
+;
+
+  // No warnings expected.
+  PrivateEmpty private_empty;
+  if (private_empty.size() == 0)
+;
+  if (private_empty.size() != 0)
+;
+  if (0 == private_empty.size())
+;
+  if (0 != private_empty.size())
+;
+  if (private_empty.size() > 0)
+;
+  if (0 < private_empty.size())
+;
+  if (private_empty.size() < 1)
+;
+  if (1 > private_empty.size())
+;
+  if (private_empty.size() >= 1)
+;
+  if (1 <= private_empty.size())
+;
+  if (private_empty.size() > 1)
+;
+  if (1 < private_empty.size())
+;
+  if (private_empty.size() <= 1)
+;
+  if (1 >= private_empty.size())
+;
+  if (!private_empty.size())
+;
+  if (private_empty.size())
+  

r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 01:51:11 2016
New Revision: 281198

URL: http://llvm.org/viewvc/llvm-project?rev=281198=rev
Log:
Add virtual destructor (necessary due to the switch to shared_ptr).

Modified:
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281198=281197=281198=diff
==
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12 01:51:11 
2016
@@ -911,6 +911,7 @@ namespace {
 struct DiagText {
   struct Piece {
 virtual void print(std::vector ) = 0;
+virtual ~Piece() {}
   };
   struct TextPiece : Piece {
 StringRef Role;


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

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Jonas Hahnfeld via cfe-commits]

Hahnfeld added a reviewer: beanz.
Hahnfeld added a comment.

SGTM, I don't see any drawback



Comment at: cmake/Modules/CompilerRTUtils.cmake:200
@@ -199,5 +199,3 @@
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 LLVM_OBJ_ROOT)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)

Is `BINARY_DIR` reserved in CMake? Otherwise I would prefer to name this 
consistently...


https://reviews.llvm.org/D24005



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

r281195 - Attempt #2 to placate MSVC

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 01:23:26 2016
New Revision: 281195

URL: http://llvm.org/viewvc/llvm-project?rev=281195=rev
Log:
Attempt #2 to placate MSVC

Modified:
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281195=281194=281195=diff
==
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12 01:23:26 
2016
@@ -929,6 +929,8 @@ struct DiagText {
   std::vector Pieces;
 
   DiagText();
+  DiagText(DiagText &) : Pieces(std::move(O.Pieces)) {}
+
   DiagText(StringRef Text);
   DiagText(StringRef Kind, StringRef Text);
 


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

Re: [PATCH] D24069: Document option '-rtlib' in clang's man page and help info

[Jonas Hahnfeld via cfe-commits]

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

LGTM, thanks for the improvement!


https://reviews.llvm.org/D24069



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

r281194 - Attempt to placate MSVC.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 01:13:44 2016
New Revision: 281194

URL: http://llvm.org/viewvc/llvm-project?rev=281194=rev
Log:
Attempt to placate MSVC.

Modified:
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281194=281193=281194=diff
==
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12 01:13:44 
2016
@@ -928,7 +928,7 @@ struct DiagText {
 
   std::vector Pieces;
 
-  DiagText() {}
+  DiagText();
   DiagText(StringRef Text);
   DiagText(StringRef Kind, StringRef Text);
 
@@ -1021,6 +1021,8 @@ DiagText parseDiagText(StringRef ,
   return Parsed;
 }
 
+DiagText::DiagText() {}
+
 DiagText::DiagText(StringRef Text) : DiagText(parseDiagText(Text, false)) {}
 
 DiagText::DiagText(StringRef Kind, StringRef Text) : 
DiagText(parseDiagText(Text, false)) {


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

r281197 - Attempt #3 to placate MSVC.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 01:38:31 2016
New Revision: 281197

URL: http://llvm.org/viewvc/llvm-project?rev=281197=rev
Log:
Attempt #3 to placate MSVC.

Modified:
cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp

Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281197=281196=281197=diff
==
--- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12 01:38:31 
2016
@@ -926,16 +926,18 @@ struct DiagText {
 void print(std::vector ) override;
   };
 
-  std::vector Pieces;
+  // FIXME: This should be a unique_ptr, but I can't figure out how to get MSVC
+  // to not issue errors on that.
+  std::vector Pieces;
 
   DiagText();
-  DiagText(DiagText &) : Pieces(std::move(O.Pieces)) {}
+  DiagText(DiagText &) LLVM_NOEXCEPT : Pieces(std::move(O.Pieces)) {}
 
   DiagText(StringRef Text);
   DiagText(StringRef Kind, StringRef Text);
 
   template void add(P Piece) {
-Pieces.push_back(llvm::make_unique(std::move(Piece)));
+Pieces.push_back(std::make_shared(std::move(Piece)));
   }
   void print(std::vector );
 };
@@ -1032,7 +1034,7 @@ DiagText::DiagText(StringRef Kind, Strin
   Prefix.Role = Kind;
   Prefix.Text = Kind;
   Prefix.Text += ": ";
-  Pieces.insert(Pieces.begin(), 
llvm::make_unique(std::move(Prefix)));
+  Pieces.insert(Pieces.begin(), 
std::make_shared(std::move(Prefix)));
 }
 
 void escapeRST(StringRef Str, std::string ) {


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

Re: [PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Daniel Marjamäki via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281206: [clang-tidy] readability-misplaced-array-index: add 
new check that warns when… (authored by danielmarjamaki).

Changed prior to commit:
  https://reviews.llvm.org/D21134?vs=69558=70994#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D21134

Files:
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
  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
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-misplaced-array-index.rst
  clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s readability-misplaced-array-index %t
+
+#define ABC  "abc"
+
+struct XY { int *X; int *Y; };
+
+void dostuff(int);
+
+void unusualSyntax(int *P1, struct XY *P2) {
+  10[P1] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression, usually the index is inside the []
+  // CHECK-FIXES: P1[10] = 0;
+
+  10[P2->X] = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: confusing array subscript expression
+  // CHECK-FIXES: P2->X[10] = 0;
+
+  dostuff(1["abc"]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+  // CHECK-FIXES:  dostuff("abc"[1]);
+
+  dostuff(1[ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+  // CHECK-FIXES:  dostuff(ABC[1]);
+
+  dostuff(0[0 + ABC]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: confusing array subscript expression
+  // CHECK-FIXES:  dostuff(0[0 + ABC]);
+  // No fixit. Probably the code should be ABC[0]
+}
+
+void normalSyntax(int *X) {
+  X[10] = 0;
+}
Index: clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.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_MISPLACED_ARRAY_INDEX_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Warn about unusual array index syntax (`index[array]` instead of
+/// `array[index]`).
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misplaced-array-index.html
+class MisplacedArrayIndexCheck : public ClangTidyCheck {
+public:
+  MisplacedArrayIndexCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISPLACED_ARRAY_INDEX_H
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
@@ -19,6 +19,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
@@ -54,6 +55,8 @@
 "readability-implicit-bool-cast");
 CheckFactories.registerCheck(
 "readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-misplaced-array-index");
 CheckFactories.registerCheck(
 "readability-static-definition-in-anonymous-namespace");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt

[clang-tools-extra] r281206 - [clang-tidy] readability-misplaced-array-index: add new check that warns when array index is misplaced.

[Daniel Marjamaki via cfe-commits]

Author: danielmarjamaki
Date: Mon Sep 12 07:04:13 2016
New Revision: 281206

URL: http://llvm.org/viewvc/llvm-project?rev=281206=rev
Log:
[clang-tidy] readability-misplaced-array-index: add new check that warns when 
array index is misplaced.

Reviewers: alexfh

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


Added:
clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-misplaced-array-index.rst

clang-tools-extra/trunk/test/clang-tidy/readability-misplaced-array-index.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=281206=281205=281206=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Mon Sep 12 
07:04:13 2016
@@ -10,6 +10,7 @@ add_clang_library(clangTidyReadabilityMo
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp?rev=281206=auto
==
--- clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.cpp 
Mon Sep 12 07:04:13 2016
@@ -0,0 +1,57 @@
+//===--- MisplacedArrayIndexCheck.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 "MisplacedArrayIndexCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void MisplacedArrayIndexCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(arraySubscriptExpr(hasLHS(hasType(isInteger())),
+hasRHS(hasType(isAnyPointer(
+ .bind("expr"),
+ this);
+}
+
+void MisplacedArrayIndexCheck::check(const MatchFinder::MatchResult ) {
+  const auto *ArraySubscriptE =
+  Result.Nodes.getNodeAs("expr");
+
+  auto Diag = diag(ArraySubscriptE->getLocStart(), "confusing array subscript "
+   "expression, usually the "
+   "index is inside the []");
+
+  // Only try to fixit when LHS and RHS can be swapped directly without 
changing
+  // the logic.
+  const Expr *RHSE = ArraySubscriptE->getRHS()->IgnoreParenImpCasts();
+  if (!isa(RHSE) && !isa(RHSE) &&
+  !isa(RHSE))
+return;
+
+  const StringRef LText = tooling::fixit::getText(
+  ArraySubscriptE->getLHS()->getSourceRange(), *Result.Context);
+  const StringRef RText = tooling::fixit::getText(
+  ArraySubscriptE->getRHS()->getSourceRange(), *Result.Context);
+
+  Diag << FixItHint::CreateReplacement(
+  ArraySubscriptE->getLHS()->getSourceRange(), RText);
+  Diag << FixItHint::CreateReplacement(
+  ArraySubscriptE->getRHS()->getSourceRange(), LText);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h?rev=281206=auto
==
--- clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h 
(added)
+++ clang-tools-extra/trunk/clang-tidy/readability/MisplacedArrayIndexCheck.h 
Mon Sep 12 07:04:13 2016
@@ -0,0 +1,36 @@
+//===--- MisplacedArrayIndexCheck.h - clang-tidy-*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See 

Re: [PATCH] D18462: Fix for clang_Cursor_getSpellingNameRange()

[Manuel Klimek via cfe-commits]

klimek added a reviewer: bkramer.
klimek added a comment.

+Benjamin


https://reviews.llvm.org/D18462



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

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Sam Shepperd via cfe-commits]

phabricss added a subscriber: phabricss.
phabricss added a comment.

It is still impossible to compile glibc with clang due to this bug.  Could this 
patch be reviewed please.


https://reviews.llvm.org/D16171



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

Re: [PATCH] D23921: Remove va_start diagnostic false positive with enumerations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D23921#531883, @aaron.ballman wrote:

> Ping


Ping


https://reviews.llvm.org/D23921



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

Re: [PATCH] D24238: StaticAnalyzer CastToStruct : No memory corruption when casting array to struct

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Random thoughts:

- This checker doesn't alter the exploded graph, so it cannot be causing or 
suppressing positives in other checkers.

- We should not be adding platform-specific behavior (eg. working as if 
`sizeof(int) == 1`) without actually ensuring that it is so on that platform.

- As far as i understand, even if `sizeof(int) == 1`, casting an `int *` into a 
structure pointer violates the strict aliasing rule. Simply because `int` and 
`char` are different types, even if they have same size and signedness (note 
that also `char`, `signed char`, `unsigned char` are three different types, 
despite two of them refer to the same thing).

> In general, using char instead of short/int does not prevent 
> alignment/endianness problems as far as I see.


You're right on this one, it'd only prevent endianness of the buffer ints from 
causing problems.


https://reviews.llvm.org/D24238



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

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Daniel Jasper via cfe-commits]

djasper added a comment.

Ok.. Thought some more and discussed with Manuel.

I think we should do a partial solution for now. I still think addOrMerge is 
harmful and it is always never the right thing to use. The codepaths that 
currently implement some version of it, should likely reconsider.

What I think we should implement is that some combinations of inserts and 
replacements are accepted by "add". Specifically, imagine you have an original 
text "aa" and a replacement that replaces it to "bb". Now, imagine you have an 
additional insert of "c" at offsets 0, 1 or 2:

- Offset 0: Not ambiguous, result should be "cbb".
- Offset 1: Conflict, report error.
- Offset 2: Not ambiguous, result should be "bbc"

So basically, inserts adjacent to replacements (including deletions) have a 
well defined order and should be supported.

Two replacements still need a defined order and we should probably error for 
now. I have some ideas on how to solve this. The most intuitive one might be to 
have a pair functions "StringRef getReplacement(unsigned offset)"/"void 
setReplacement(unsigned offset, StringRef text)", that control the replacement 
at a given offset. A replacement object shouldn't have more than one insert at 
any given offset. But again, I think that's a problem we can get to later. I 
think most tools that currently would insert stuff at the same location are 
actually wrong and we should continue to error.

Similarly, we never want to merge overlapping replacements, but we should 
ensure that directly adjacent ones work, e.g. "aa" can be convert into "bc" 
with two replacements (one for the first character, one for the second) that 
are combined with add().

What do you think?


https://reviews.llvm.org/D24383



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

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Meike Baumgärtner via cfe-commits]

meikeb marked 4 inline comments as done.


Comment at: lib/Sema/SemaChecking.cpp:4166-4167
@@ +4165,4 @@
+if (BinOp->isAdditiveOp()) {
+  bool LIsInt = false;
+  bool RIsInt = false;
+  // Prevent asserts triggering in EvaluateAsInt by checking if we deal 
with

I hope this additional check fixes this issue. As far as I read the code there 
were none such asserts in isIntegerConstantExpr(). Thanks for explaining this!


https://reviews.llvm.org/D23820



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

Re: r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

[David Blaikie via cfe-commits]

Ah, hmm - we do use this technique in a few places. Oh, right - protected
in the base, final derived classes (maybe that's the missing element?). So
I think we've disabled whatever GCC warnings get in the way of that
particular cliche.

But, fair enough - thanks for the context. All power to the MSVC-compatible
engines.

On Mon, Sep 12, 2016 at 10:57 AM Richard Smith 
wrote:

> On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Does that actually need a virtual dtor? The type erasure handling in
> shared_ptr would handle it so long as each instance is constructed with the
> derived type (looks like it probably is - make_shared used consistently) &
> we should get a warning (or could make the base dtor protected non-virtual
> to ensure an error) if we miss that?
>
>
> We get warnings from GCC regardless, and that broke a few bots :( I'd like
> to switch this back to unique_ptr at some point (once I've figured out why
> MSVC rejected it) and at that point we'll need the virtual destructor.
>
> But I can understand that it's perhaps nice to do even if we could thread
> that delicate needle. Just curious.
>
> On Sun, Sep 11, 2016 at 11:59 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: rsmith
> Date: Mon Sep 12 01:51:11 2016
> New Revision: 281198
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281198=rev
> Log:
> Add virtual destructor (necessary due to the switch to shared_ptr).
>
> Modified:
> cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>
> Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281198=281197=281198=diff
>
> ==
> --- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12
> 01:51:11 2016
> @@ -911,6 +911,7 @@ namespace {
>  struct DiagText {
>struct Piece {
>  virtual void print(std::vector ) = 0;
> +virtual ~Piece() {}
>};
>struct TextPiece : Piece {
>  StringRef Role;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r281057 - [DebugInfo] Ensure complete type is emitted with -fstandalone-debug

[David Blaikie via cfe-commits]

Looks like this test could be a bit simpler:

struct foo;
foo *f; // produces a forward decl
struct foo {
  virtual ~foo();
};
foo f; // requires the type to be complete

A cursory glance looks like this reproduces the issue, but I may've missed
something.

Also the bracing in the new condition you added is inconsistent with the
existing bracing - could you fix that? (add braces to the middle if,
following the "any block more than one line should have braces", or remove
them from the outer one you added (following the "any block of a single
statement should omit braces"))

(idle thought: It'd be good to make the code common between the first time
a type is encountered, and the later callbacks when the type is completed,
required to be complete, vtable is emitted, etc - the inconsistency here (&
it's spread out through several functions: complete[Required]Type,
shouldOmitDefinition, isDefinedInClangModule, etc) is a bit
problematic/error prone)

On Fri, Sep 9, 2016 at 10:12 AM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Fri Sep  9 12:03:53 2016
> New Revision: 281057
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281057=rev
> Log:
> [DebugInfo] Ensure complete type is emitted with -fstandalone-debug
>
> The logic for upgrading a class from a forward decl to a complete type
> was not checking the debug info emission level before applying the
> vtable optimization. This meant we ended up without debug info for a
> class which was required to be complete. I noticed it because it
> triggered an assertion during CodeView emission, but that's a separate
> issue.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281057=281056=281057=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Sep  9 12:03:53 2016
> @@ -1648,9 +1648,13 @@ void CGDebugInfo::completeRequiredType(c
>if (DebugKind <= codegenoptions::DebugLineTablesOnly)
>  return;
>
> -  if (const auto *CXXDecl = dyn_cast(RD))
> -if (CXXDecl->isDynamicClass())
> -  return;
> +  // If this is a dynamic class and we're emitting limited debug info,
> wait
> +  // until the vtable is emitted to complete the class debug info.
> +  if (DebugKind <= codegenoptions::LimitedDebugInfo) {
> +if (const auto *CXXDecl = dyn_cast(RD))
> +  if (CXXDecl->isDynamicClass())
> +return;
> +  }
>
>if (DebugTypeExtRefs && RD->isFromASTFile())
>  return;
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281057=281056=281057=diff
>
> ==
> --- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Fri Sep  9
> 12:03:53 2016
> @@ -1,8 +1,29 @@
> -// RUN: %clang_cc1 -triple x86_64-unk-unk -debug-info-kind=standalone -o
> - -emit-llvm %s | FileCheck %s
> -// On Darwin, "full" debug info is the default, so really these tests are
> -// identical, as cc1 no longer chooses the effective value of
> DebugInfoKind.
>  // RUN: %clang_cc1 -triple x86_64-apple-darwin
> -debug-info-kind=standalone -o - -emit-llvm %s | FileCheck %s
>
> +// We had a bug in -fstandalone-debug where UnicodeString would not be
> completed
> +// when it was required to be complete. This orginally manifested as an
> +// assertion in CodeView emission on Windows with some dllexport stuff,
> but it's
> +// more general than that.
> +
> +struct UnicodeString;
> +struct GetFwdDecl {
> +  static UnicodeString format;
> +};
> +GetFwdDecl force_fwd_decl;
> +struct UnicodeString {
> +private:
> +  virtual ~UnicodeString();
> +};
> +struct UseCompleteType {
> +  UseCompleteType();
> +  ~UseCompleteType();
> +  UnicodeString currencySpcAfterSym[1];
> +};
> +UseCompleteType require_complete;
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name:
> "UnicodeString"
> +// CHECK-NOT:  DIFlagFwdDecl
> +// CHECK-SAME: ){{$}}
> +
>  namespace rdar14101097_1 { // see also PR16214
>  // Check that we emit debug info for the definition of a struct if the
>  // definition is available, even if it's used via a pointer wrapped in a
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Meike Baumgärtner via cfe-commits]

meikeb updated this revision to Diff 71043.
meikeb added a comment.

Fix the mentioned issues.


https://reviews.llvm.org/D23820

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/format-strings.c

Index: test/Sema/format-strings.c
===
--- test/Sema/format-strings.c
+++ test/Sema/format-strings.c
@@ -652,3 +652,38 @@
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"
+
+void test_char_pointer_arithmetic(int b) {
+  const char s1[] = "string";
+  const char s2[] = "%s string";
+
+  printf(s1 - 1);  // expected-warning {{format string is not a string literal (potentially insecure)}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
+
+  printf(s1 + 2);  // no-warning
+  printf(s2 + 2);  // no-warning
+
+  const char s3[] = "%s string";
+  printf((s3 + 2) - 2);  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + s2); // no-warning
+  printf(6 + s2 - 2); // no-warning
+  printf(2 + (b ? s1 : s2));  // no-warning
+
+  const char s5[] = "string %s";
+  printf(2 + (b ? s2 : s5));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + (b ? s2 : s5), "");  // no-warning
+  printf(2 + (b ? s1 : s2 - 2), "");  // no-warning
+
+  const char s6[] = "%s string";
+  printf(2 + (b ? s1 : s6 - 2));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(1 ? s2 + 2 : s2);  // no-warning
+  printf(0 ? s2 : s2 + 2);  // no-warning
+  printf(2 + s2 + 5 * 3 - 16, "");  // expected-warning{{data argument not used}}
+
+  const char s7[] = "%s string %s %s";
+  printf(s7 + 3, "");  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3839,7 +3839,92 @@
 };
 } // end anonymous namespace
 
-static void CheckFormatString(Sema , const StringLiteral *FExpr,
+static void reckonUpOffset(llvm::APSInt , llvm::APSInt Addend,
+   BinaryOperatorKind BinOpKind, bool AddendIsRight) {
+  unsigned BitWidth = Offset.getBitWidth();
+  unsigned AddendBitWidth = Addend.getBitWidth();
+  // There might be negative interim results.
+  if (Addend.isUnsigned()) {
+Addend = Addend.zext(++AddendBitWidth);
+Addend.setIsSigned(true);
+  }
+  // Align the bit width of the APSInts.
+  if (AddendBitWidth > BitWidth) {
+Offset = Offset.sext(AddendBitWidth);
+BitWidth = AddendBitWidth;
+  } else if (BitWidth > AddendBitWidth) {
+Addend = Addend.sext(BitWidth);
+  }
+
+  bool Ov = false;
+  llvm::APSInt ResOffset = Offset;
+  if (BinOpKind == BO_Add)
+ResOffset = Offset.sadd_ov(Addend, Ov);
+  else if (AddendIsRight && BinOpKind == BO_Sub)
+ResOffset = Offset.ssub_ov(Addend, Ov);
+
+  // We add a offset to a pointer here so we should support a offset as big as
+  // possible.
+  if (Ov) {
+assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
+Offset.sext(2 * BitWidth);
+reckonUpOffset(Offset, Addend, BinOpKind, AddendIsRight);
+return;
+  }
+
+  Offset = ResOffset;
+}
+
+namespace {
+// This is a wrapper class around StringLiteral to support offsetted string
+// literals as format strings. It takes the offset into account when retruning
+// the string and its length or the source locations to display notes correctly.
+class FormatStringLiteral {
+  const StringLiteral *FExpr;
+  int64_t Offset;
+
+ public:
+  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
+  : FExpr(fexpr), Offset(Offset) {}
+
+  StringRef getString() const {
+return StringRef(FExpr->getString().data() + Offset,
+ FExpr->getLength() - Offset);
+  }
+
+  unsigned getByteLength() const {
+return FExpr->getByteLength() - getCharByteWidth() * Offset;
+  }
+  unsigned getLength() const { return FExpr->getLength() - Offset; }
+  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
+
+  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
+
+  QualType getType() const { return FExpr->getType(); }
+
+  bool isAscii() const { return FExpr->isAscii(); }
+  bool isWide() const { return FExpr->isWide(); }
+  bool isUTF8() const { return FExpr->isUTF8(); }
+  bool isUTF16() const { return FExpr->isUTF16(); }
+  bool isUTF32() const { return FExpr->isUTF32(); }
+  bool isPascal() const { return FExpr->isPascal(); }
+
+  SourceLocation getLocationOfByte(
+  unsigned ByteNo, const SourceManager , const LangOptions ,
+  const TargetInfo , unsigned *StartToken = nullptr,
+  

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Chris Bieneman via cfe-commits]

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

One comment below, otherwise LGTM.



Comment at: cmake/Modules/CompilerRTUtils.cmake:200
@@ -199,5 +199,3 @@
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 LLVM_OBJ_ROOT)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)

mgorny wrote:
> Hahnfeld wrote:
> > Is `BINARY_DIR` reserved in CMake? Otherwise I would prefer to name this 
> > consistently...
> I don't think it is. I used that name for consistency with llvm-config 
> options (naming --obj-root BINARY_DIR actually confused me a lot at a first 
> glance), and for consistency with the code used in clang (which names 
> variables exactly like this).
> 
> Not saying I like changing names like this but I think if any change is due, 
> we should do it everywhere consistently.
If you want to be consistent with the options (which makes sense), you should 
name it `OBJ_ROOT`, and also rename `TOOLS_BINARY_DIR` to `BINDIR`, 
`LIBRARY_DIR` to `LIBDIR`, and `MAIN_SRC_DIR` to `SRC_ROOT`. Being partially 
consistent doesn't make sense.


https://reviews.llvm.org/D24005



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

[PATCH] D24467: Fix an error after D21678

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl created this revision.
vbyakovlcl added reviewers: ahatanak, aaron.ballman.
vbyakovlcl added subscribers: andreybokhanko, cfe-commits.
vbyakovlcl set the repository for this revision to rL LLVM.
vbyakovlcl changed the visibility of this Differential Revision from "Public 
(No Login Required)" to "All Users".
Herald added a subscriber: aemerson.

This fixes an error and gcc incompatibilities in vector shifts reported by 
Akira Hatanaka

--
From: Akira Hatanaka 
Date: Fri, Sep 2, 2016 at 3:00 AM
Subject: Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector 
values
To: vladimi...@gmail.com, ulrich.weig...@de.ibm.com, amjad.ab...@intel.com, 
guy.ben...@intel.com, aaron.ball...@gmail.com
Cc: ahata...@gmail.com, andreybokha...@gmail.com, anastasia.stul...@arm.com, 
dmitry.poluk...@gmail.com, cfe-commits@lists.llvm.org


ahatanak added a subscriber: ahatanak.
ahatanak added a comment.

This patch causes clang to error out on the following code, which used to 
compile fine:

$ cat f2.c

  typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;

  vector_ushort8 foo1(void) {
return 1 << (vector_ushort8){7,6,5,4,3,2,1,0};
  }

$ clang f2.c -c

clang used to transform the scaler operand to a vector operand (similar to the 
way gcc's vector is handled) when compiling for normal c/c++ (but printed an 
error message when compiling for opencl), but this patch dropped the check for 
LangOpts added in r230464 and changed that behavior. I don't think this was 
intentional?


Repository:
  rL LLVM

https://reviews.llvm.org/D21678

Repository:
  rL LLVM

https://reviews.llvm.org/D24467

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  llvm/tools/clang/lib/Sema/SemaExpr.cpp
  llvm/tools/clang/test/Sema/vecshift.c

Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8721,24 +8721,28 @@
 static QualType checkVectorShift(Sema , ExprResult , ExprResult ,
  SourceLocation Loc, bool IsCompAssign) {
   // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
-  if (!LHS.get()->getType()->isVectorType()) {
+  if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) &&
+  !LHS.get()->getType()->isVectorType()) {
 S.Diag(Loc, diag::err_shift_rhs_only_vector)
   << RHS.get()->getType() << LHS.get()->getType()
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 return QualType();
   }
 
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());
+else
+  LHS = S.DefaultFunctionArrayLvalueConversion(LHS.get());
 if (LHS.isInvalid()) return QualType();
   }
 
   RHS = S.UsualUnaryConversions(RHS.get());
   if (RHS.isInvalid()) return QualType();
 
   QualType LHSType = LHS.get()->getType();
-  const VectorType *LHSVecTy = LHSType->castAs();
-  QualType LHSEleType = LHSVecTy->getElementType();
+  const VectorType *LHSVecTy = LHSType->getAs();
+  QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType;
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();
@@ -8758,7 +8762,19 @@
 return QualType();
   }
 
-  if (RHSVecTy) {
+  if (!LHSVecTy) {
+assert(RHSVecTy);
+if (IsCompAssign)
+  return RHSType;
+if (LHSEleType != RHSEleType) {
+  LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast);
+  LHSEleType = RHSEleType;
+}
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
+LHSType = VecTy;
+  } else if (RHSVecTy) {
 // OpenCL v1.1 s6.3.j says that for vector types, the operators
 // are applied component-wise. So if RHS is a vector, then ensure
 // that the number of elements is the same as LHS...
@@ -8768,6 +8784,62 @@
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
   return QualType();
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();
+  const BuiltinType *RHSBT = RHSEleType->getAs();
+  if (LHSBT != RHSBT) {
+BuiltinType::Kind LHSKind = LHSBT->getKind();
+BuiltinType::Kind RHSKind = RHSBT->getKind();
+bool DiffSizes = true;
+switch (LHSKind) {
+case BuiltinType::Char_S:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_U && RHSKind != BuiltinType::UChar;
+  break;
+case BuiltinType::Char_U:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_S && RHSKind != BuiltinType::UChar;
+  break;
+case BuiltinType::UChar:
+

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Michał Górny via cfe-commits]

mgorny marked 3 inline comments as done.
mgorny added a comment.

And done.


https://reviews.llvm.org/D24005



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

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Michał Górny via cfe-commits]

mgorny updated this revision to Diff 71040.
Herald added subscribers: mgorny, beanz.

https://reviews.llvm.org/D24005

Files:
  cmake/Modules/CompilerRTUtils.cmake

Index: cmake/Modules/CompilerRTUtils.cmake
===
--- cmake/Modules/CompilerRTUtils.cmake
+++ cmake/Modules/CompilerRTUtils.cmake
@@ -197,10 +197,15 @@
 message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
   endif()
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 BINARY_DIR)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+  list(GET CONFIG_OUTPUT 3 MAIN_SRC_DIR)
+
+  set(LLVM_BINARY_DIR ${BINARY_DIR} CACHE PATH "Path to LLVM build tree")
+  set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
+  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
+  set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
   # Make use of LLVM CMake modules.
   file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE)


Index: cmake/Modules/CompilerRTUtils.cmake
===
--- cmake/Modules/CompilerRTUtils.cmake
+++ cmake/Modules/CompilerRTUtils.cmake
@@ -197,10 +197,15 @@
 message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
   endif()
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT ${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 BINARY_DIR)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+  list(GET CONFIG_OUTPUT 3 MAIN_SRC_DIR)
+
+  set(LLVM_BINARY_DIR ${BINARY_DIR} CACHE PATH "Path to LLVM build tree")
+  set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
+  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
+  set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
   # Make use of LLVM CMake modules.
   file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: lib/Sema/SemaChecking.cpp:3842-3843
@@ -3841,2 +3841,4 @@
 
-static void CheckFormatString(Sema , const StringLiteral *FExpr,
+static void reckonUpOffset(llvm::APSInt , llvm::APSInt Addend,
+   BinaryOperatorKind BinOpKind, bool AddendIsRight) {
+  unsigned BitWidth = Offset.getBitWidth();

I can't tell from this declaration what this function is for -- what does 
"reckon up" mean?


Comment at: lib/Sema/SemaChecking.cpp:3880
@@ +3879,3 @@
+// This is a wrapper class around StringLiteral to support offsetted string
+// literals as format strings. It takes the offset into account when retruning
+// the string and its length or the source locations to display notes 
correctly.

Typo "retruning"


Comment at: lib/Sema/SemaChecking.cpp:3891-3892
@@ +3890,4 @@
+  StringRef getString() const {
+return StringRef(FExpr->getString().data() + Offset,
+ FExpr->getLength() - Offset);
+  }

I think you can simplify this to `return FExpr->getString().drop_front(Offset);`


Comment at: lib/Sema/SemaChecking.cpp:4143
@@ -4049,2 +4142,3 @@
 if (StrE) {
-  CheckFormatString(S, StrE, E, Args, HasVAListArg, format_idx,
+  if (Offset.isNegative()) {
+// TODO: It would be better to have an explicit warning for out of

You should presumably also do this if `Offset` is >= the length of the string 
literal (we want `printf` and friends to at least find the trailing nul byte).


Comment at: lib/Sema/SemaChecking.cpp:4148-4149
@@ +4147,4 @@
+  }
+  FormatStringLiteral *FStr =
+  new FormatStringLiteral(StrE, Offset.sextOrTrunc(64).getSExtValue());
+  CheckFormatString(S, FStr, E, Args, HasVAListArg, format_idx,

Does this need to be heap-allocated?


Comment at: lib/Sema/SemaChecking.cpp:4166-4167
@@ +4165,4 @@
+if (BinOp->isAdditiveOp()) {
+  bool LIsInt = false;
+  bool RIsInt = false;
+  // Prevent asserts triggering in EvaluateAsInt by checking if we deal 
with

meikeb wrote:
> I hope this additional check fixes this issue. As far as I read the code 
> there were none such asserts in isIntegerConstantExpr(). Thanks for 
> explaining this!
OK, I've now checked and the value-dependent case is handled up on line 3953. 
So just calling `EvaluateAsInt` here is fine after all.


https://reviews.llvm.org/D23820



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

[PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Edward Jones via cfe-commits]

edward-jones created this revision.
edward-jones added a reviewer: rjmccall.
edward-jones added a subscriber: cfe-commits.

llvm.flt.rounds returns an i32, but the builtin expects an integer. On targets 
where integers are not 32-bits clang tries to bitcast the result, causing an 
assertion failure.

https://reviews.llvm.org/D24461

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins-msp430.c
  test/CodeGen/builtins.c

Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -227,6 +227,9 @@
   // CHECK: fcmp uge float {{.*}}, 0x3810
   // CHECK: and i1
   // CHECK: and i1
+
+  res = __builtin_flt_rounds();
+  // CHECK: call i32 @llvm.flt.rounds(
 }
 
 // CHECK-LABEL: define void @test_float_builtin_ops
Index: test/CodeGen/builtins-msp430.c
===
--- /dev/null
+++ test/CodeGen/builtins-msp430.c
@@ -0,0 +1,10 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
+
+int test_builtin_flt_rounds() {
+  // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.flt.rounds()
+  // CHECK-DAG: [[V1:[%A-Za-z0-9.]+]] = trunc i32 [[V0]] to i16
+  // CHECK-DAG: ret i16 [[V1]]
+  return __builtin_flt_rounds();
+}
+
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -953,6 +953,17 @@
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BI__builtin_flt_rounds: {
+Value *F = CGM.getIntrinsic(Intrinsic::flt_rounds);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Result = Builder.CreateCall(F);
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 Value *V = EmitScalarExpr(E->getArg(5));
 llvm::Type *Ty = ConvertType(E->getArg(5)->getType());


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -227,6 +227,9 @@
   // CHECK: fcmp uge float {{.*}}, 0x3810
   // CHECK: and i1
   // CHECK: and i1
+
+  res = __builtin_flt_rounds();
+  // CHECK: call i32 @llvm.flt.rounds(
 }
 
 // CHECK-LABEL: define void @test_float_builtin_ops
Index: test/CodeGen/builtins-msp430.c
===
--- /dev/null
+++ test/CodeGen/builtins-msp430.c
@@ -0,0 +1,10 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+int test_builtin_flt_rounds() {
+  // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.flt.rounds()
+  // CHECK-DAG: [[V1:[%A-Za-z0-9.]+]] = trunc i32 [[V0]] to i16
+  // CHECK-DAG: ret i16 [[V1]]
+  return __builtin_flt_rounds();
+}
+
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -953,6 +953,17 @@
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BI__builtin_flt_rounds: {
+Value *F = CGM.getIntrinsic(Intrinsic::flt_rounds);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Result = Builder.CreateCall(F);
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 Value *V = EmitScalarExpr(E->getArg(5));
 llvm::Type *Ty = ConvertType(E->getArg(5)->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23300: [analyzer] Add "Assuming..." diagnostic pieces for unsupported condition expressions.

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

@NoQ,

Let's test in an IDE. Can you send screenshots?


https://reviews.llvm.org/D23300



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

Re: r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

[David Blaikie via cfe-commits]

Does that actually need a virtual dtor? The type erasure handling in
shared_ptr would handle it so long as each instance is constructed with the
derived type (looks like it probably is - make_shared used consistently) &
we should get a warning (or could make the base dtor protected non-virtual
to ensure an error) if we miss that?

But I can understand that it's perhaps nice to do even if we could thread
that delicate needle. Just curious.

On Sun, Sep 11, 2016 at 11:59 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Sep 12 01:51:11 2016
> New Revision: 281198
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281198=rev
> Log:
> Add virtual destructor (necessary due to the switch to shared_ptr).
>
> Modified:
> cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>
> Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp?rev=281198=281197=281198=diff
>
> ==
> --- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12
> 01:51:11 2016
> @@ -911,6 +911,7 @@ namespace {
>  struct DiagText {
>struct Piece {
>  virtual void print(std::vector ) = 0;
> +virtual ~Piece() {}
>};
>struct TextPiece : Piece {
>  StringRef Role;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22968: [analyzer] A checker for macOS-specific bool-like objects.

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Let's test it on more real word bugs.



Comment at: lib/StaticAnalyzer/Checkers/BoolConversionChecker.cpp:11
@@ +10,3 @@
+// This file defines BoolConversionChecker, which checks for a particular
+// common mistake when dealing with NSNumber and OSBoolean objects:
+// When having a pointer P to such object, neither `if (P)' nor `bool X = P'

Should we warn on comparisons of NSNumber to '0'? Comparisons to 'nil' would be 
considered a proper pointer comparison, while comparisons to literal '0' would 
be considered a faulty comparison of the numbers.


Comment at: lib/StaticAnalyzer/Checkers/BoolConversionChecker.cpp:49
@@ +48,3 @@
+  virtual void run(const MatchFinder::MatchResult ) {
+// This callback only throws reports. All the logic is in the matchers.
+const Stmt *Conv = Result.Nodes.getNodeAs("conv");

"throws" -> "generates"?


https://reviews.llvm.org/D22968



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

[Hans Wennborg via cfe-commits]

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

lgtm

I'm a little bit worried that we've started down a slippery slope of deciding 
what is and isn't allowed in naked functions (for example, some initializers 
could be allowed if we're sure they don't require stack). But I suppose this 
isn't making anything worse, and if it helps compile real-world code, that's 
good.


https://reviews.llvm.org/D24193



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

r281241 - Diagnostics reference: "error:" should be red, not orange.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 12:55:49 2016
New Revision: 281241

URL: http://llvm.org/viewvc/llvm-project?rev=281241=rev
Log:
Diagnostics reference: "error:" should be red, not orange.

Modified:
cfe/trunk/docs/DiagnosticsReference.rst
cfe/trunk/include/clang/Basic/DiagnosticDocs.td

Modified: cfe/trunk/docs/DiagnosticsReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/DiagnosticsReference.rst?rev=281241=281240=281241=diff
==
--- cfe/trunk/docs/DiagnosticsReference.rst (original)
+++ cfe/trunk/docs/DiagnosticsReference.rst Mon Sep 12 12:55:49 2016
@@ -25,7 +25,7 @@
 .error {
   font-family: monospace;
   font-weight: bold;
-  color: #c70;
+  color: #c00;
 }
 .warning {
   font-family: monospace;

Modified: cfe/trunk/include/clang/Basic/DiagnosticDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDocs.td?rev=281241=281240=281241=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDocs.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDocs.td Mon Sep 12 12:55:49 2016
@@ -35,7 +35,7 @@ def GlobalDocumentation {
 .error {
   font-family: monospace;
   font-weight: bold;
-  color: #c70;
+  color: #c00;
 }
 .warning {
   font-family: monospace;


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

r281227 - Trivial documentation fix regarding Obj-C ARC objc_arc_weak_reference_unavailable

[Jonathan Roelofs via cfe-commits]

Author: jroelofs
Date: Mon Sep 12 11:14:52 2016
New Revision: 281227

URL: http://llvm.org/viewvc/llvm-project?rev=281227=rev
Log:
Trivial documentation fix regarding Obj-C ARC 
objc_arc_weak_reference_unavailable

Fixed incorrect docs that referred to:
  objc_arc_weak_unavailable
when it should be:
  objc_arc_weak_reference_unavailable

Patch by: Sean McBride!

Modified:
cfe/trunk/docs/AutomaticReferenceCounting.rst

Modified: cfe/trunk/docs/AutomaticReferenceCounting.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/AutomaticReferenceCounting.rst?rev=281227=281226=281227=diff
==
--- cfe/trunk/docs/AutomaticReferenceCounting.rst (original)
+++ cfe/trunk/docs/AutomaticReferenceCounting.rst Mon Sep 12 11:14:52 2016
@@ -910,10 +910,10 @@ not support ``__weak`` references.
   binary compatibility.
 
 A class may indicate that it does not support weak references by providing the
-``objc_arc_weak_unavailable`` attribute on the class's interface declaration.  
A
+``objc_arc_weak_reference_unavailable`` attribute on the class's interface 
declaration.  A
 retainable object pointer type is **weak-unavailable** if
 is a pointer to an (optionally protocol-qualified) Objective-C class ``T`` 
where
-``T`` or one of its superclasses has the ``objc_arc_weak_unavailable``
+``T`` or one of its superclasses has the 
``objc_arc_weak_reference_unavailable``
 attribute.  A program is ill-formed if it applies the ``__weak`` ownership
 qualifier to a weak-unavailable type or if the value operand of a weak
 assignment operation has a weak-unavailable type.


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

Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Daniel Krupp via cfe-commits]

dkrupp marked 11 inline comments as done.
dkrupp added a comment.

issues fixed


https://reviews.llvm.org/D24307



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

Re: r281056 - Make -fstandalone-debug and -flimit-debug-info available in clang-cl

[David Blaikie via cfe-commits]

standalone and limit-debug-info are the same functionality -
-fstandalone-debug is the more modern and apt description, so perhaps just
keep that one & drop/don't provide -flimit-debug-info (since there's no
real need to be backwards compatible in the flag support in clang-cl, I
expect)?

On Fri, Sep 9, 2016 at 9:51 AM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Fri Sep  9 11:42:50 2016
> New Revision: 281056
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281056=rev
> Log:
> Make -fstandalone-debug and -flimit-debug-info available in clang-cl
>
> Our limited debug info optimizations are breaking down at DLL
> boundaries, so we're going to evaluate the size impact of these
> settings, and possibly change the default.
>
> Users should be able to override our settings, though.
>
> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/test/Driver/cl-options.c
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=281056=281055=281056=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Fri Sep  9 11:42:50 2016
> @@ -1138,12 +1138,12 @@ def fstack_protector_strong : Flag<["-"]
>HelpText<"Use a strong heuristic to apply stack protectors to
> functions">;
>  def fstack_protector : Flag<["-"], "fstack-protector">, Group,
>HelpText<"Enable stack protectors for functions potentially vulnerable
> to stack smashing">;
> -def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group,
> +def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group,
> Flags<[CoreOption]>,
>HelpText<"Emit full debug info for all types used by the program">;
> -def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">,
> Group,
> +def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">,
> Group, Flags<[CoreOption]>,
>HelpText<"Limit debug information produced to reduce size of debug
> binary">;
> -def flimit_debug_info : Flag<["-"], "flimit-debug-info">,
> Alias;
> -def fno_limit_debug_info : Flag<["-"], "fno-limit-debug-info">,
> Alias;
> +def flimit_debug_info : Flag<["-"], "flimit-debug-info">,
> Flags<[CoreOption]>, Alias;
> +def fno_limit_debug_info : Flag<["-"], "fno-limit-debug-info">,
> Flags<[CoreOption]>, Alias;
>  def fstrict_aliasing : Flag<["-"], "fstrict-aliasing">, Group,
>Flags<[DriverOption, CoreOption]>;
>  def fstrict_enums : Flag<["-"], "fstrict-enums">, Group,
> Flags<[CC1Option]>,
>
> Modified: cfe/trunk/test/Driver/cl-options.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=281056=281055=281056=diff
>
> ==
> --- cfe/trunk/test/Driver/cl-options.c (original)
> +++ cfe/trunk/test/Driver/cl-options.c Fri Sep  9 11:42:50 2016
> @@ -518,6 +518,8 @@
>  // RUN: -resource-dir \
>  // RUN: -Wunused-variable \
>  // RUN: -fmacro-backtrace-limit=0 \
> +// RUN: -fstandalone-debug \
> +// RUN: -flimit-debug-info \
>  // RUN: -Werror /Zs -- %s 2>&1
>
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Nick Lewycky via cfe-commits]

On 11 September 2016 at 21:14, Sam Shepperd via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> phabricss added a subscriber: phabricss.
> phabricss added a comment.
>
> It is still impossible to compile glibc with clang due to this bug.  Could
> this patch be reviewed please.
>

Firstly, I thought glibc had applied a patch to fix this bug? As in, the
error is correct and glibc fixed their bug?

As for the patch, its goal is to only demote an error to warning when the
function with conflicting asm labels is never used. That's pretty clearly a
workaround (if you ever call acoshl, then you'll get an error), but
moreover the check is implemented incorrectly. "New->isUsed()" will be
performed at the moment the new declaration is being parsed; of course it
hasn't been used yet. isUsed() will turn true only after it's used, after
we actually parse the rest of the code and build AST for the uses of the
new declaration. As I suggested in my last review comment, you would need
to defer the check until ActOnEndOfTranslationUnit in order to correctly
answer New->isUsed().

Or just not try to implement this workaround, and instead change how
codegen works so that we emit calls to functions with the same asm label
that gcc would choose.

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

[PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

[Nico Weber via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.
thakis added subscribers: cfe-commits, aaron.ballman.

This mostly behaves cl.exe's behavior, even though clang-cl is stricter in some 
corner cases and more lenient in others (see the included test).

To make the `uuid declared previously here` diagnostic work correctly, tweak 
stripTypeAttributesOffDeclSpec() to keep attributes in the right order.

https://reviews.llvm.org/D24469

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCXX/ms-uuid.cpp

Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s
+
+typedef struct _GUID {
+  unsigned long Data1;
+  unsigned short Data2;
+  unsigned short Data3;
+  unsigned char Data4[8];
+} GUID;
+
+namespace {
+
+// cl.exe's behavior with merging uuid attributes is a bit erratic:
+// * In []-style attributes, a single [] list must not list a duplicate uuid
+//   (even if it's the same uuid), and only a single declaration of a class
+//   must have a uuid else the compiler errors out (even if two declarations of
+//   a class have the same uuid).
+// * For __declspec(uuid(...)), it's ok if several declarations of a class have
+//   an uuid, as long as it's the same uuid each time.  If uuids on declarations
+//   don't match, the compiler errors out.
+// * If there are several __declspec(uuid(...))s on one declaration, the
+//   compiler only warns about this and uses the last uuid.  It even warns if
+//   the uuids are the same.
+
+// clang-cl implements the following simpler (but largely compatible) behavior
+// instead:
+// * [] and __declspec uuids have the same behavior.
+// * If there are several uuids on a a class (no matter if on the same decl or
+//   on several decls), it is an error if they don't match.
+// * Having several uuids that match is ok.
+
+// Both cl and clang-cl accept this:
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("11A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("22A0---C000-0049")) C2 {};
+
+// clang-cl accepts this, but cl errors out:
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3 {};
+
+// Both cl and clang-cl error out on this (but for different reasons):
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+[uuid("11A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}}
+[uuid("22A0---C000-0049")] class C4 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049"))
+// expected-error@+1 {{uiid does not match previous declaration}}
+  __declspec(uuid("11A0---C000-0049")) C5;
+
+// XXX diag is the wrong way round here
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049"),
+// expected-error@+1 {{uiid does not match previous declaration}}
+ uuid("11A0---C000-0049")] class C6;
+
+// cl doesn't diagnose having one uuid each as []-style attributes and as
+// __declspec, even if the uuids differ.  clang-cl errors if they differ.
+[uuid("00A0---C000-0049")]
+class __declspec(uuid("00A0---C000-0049")) C7;
+
+// XXX diag order is kind of weird here
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049")]
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("11A0---C000-0049")) C8;
+
+
+// cl warns on this, but clang-cl is fine with it (which is consistent with
+// e.g. specifying __multiple_inheritance several times, which cl accepts
+// without warning too).
+class __declspec(uuid("00A0---C000-0049"))
+  

[PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added a reviewer: rizsotto.mailinglist.
dcoughlin added a subscriber: cfe-commits.

Remove the relative path hack in scan-build-py that converts a fully qualified 
directory name and a fully qualified file path to a relative path before 
running the analyzer on a file.

This hack is not needed: the bad interaction with SATestsBuild.py it was 
intended to address is actually the same underlying problem that D24163 fixed. 
Further, because the hack would always relativize paths, it caused 
SATestBuild.py to be unable to properly line up issues when the build system 
changed directory and then built a source file in a child directory but used a 
fully-qualified path for the source file.

Laszlo: Are you OK with me removing this hack? This is the last outstanding 
issue in scan-build-py preventing us in using it for our regression suite.

Note: I still think the approach used here (and especially in intercept-build) 
is not ideal. I really think we should be storing both the working directory 
and the actual path argument (even if it is relative) in the compilation 
database. Storing the fully-qualified paths loses information (i.e., which of 
the many possible paths for that file was actually used) and makes replaying 
the build with fidelity impossible. That said, after D24163 I don't have any 
examples where is actually an issue in practice.

https://reviews.llvm.org/D24470

Files:
  tools/scan-build-py/libscanbuild/runner.py
  tools/scan-build-py/tests/unit/test_runner.py

Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def 

Re: [libcxx] r281179 - [libcxx] Introduce an externally-threaded libc++ variant.

[Asiri Rathnayake via cfe-commits]

Hi Mike,

That does look related to my change. Is this an internal build? I was
looking out for the bots on llvm.org but didn't spot any issues (yet).

Will try to reproduce locally. Would it be OK if I find a fix tomorrow?
(night time here in the UK), can revert if this is blocking you.

Cheers,

/ Asiri


On Mon, Sep 12, 2016 at 9:25 PM, Mike Aizatsky via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Asiri,
>
> The boot strap build has started to fail for. Your change seems to be
> related, right?
>
> [14/22] Building CXX object projects/libcxx/lib/
> CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o
> FAILED: /usr/bin/g++   -D_DEBUG -D_GNU_SOURCE 
> -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Iprojects/libcxx/lib -I/usr/local/google/home/aizatsky/
> src/llvm/projects/libcxx/lib -Iinclude 
> -I/usr/local/google/home/aizatsky/src/llvm/include
> -I/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include -fPIC
> -fvisibility-inlines-hidden -Wall -W -Wno-unuse
> d-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers
>  -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
> -Wno-comment -std=c++11 -ffunction-sections -fdata-sections -O3 -UN
> DEBUG -DLIBCXX_BUILDING_LIBCXXABI -std=c++11 -nostdinc++ -Wall -Wextra -W
> -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type
> -Wno-attributes -Wno-literal-suffix -Wno-c++14-compat -Wno-erro
> r -fPIC -MMD -MT 
> projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o
> -MF projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o.d
> -o projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/
> src/locale.cpp.o -c /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/src/locale.cpp
> In file included from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/__mutex_base:17:0,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/mutex:189,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/__locale:18,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/locale:182,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/src/locale.cpp:17:
> /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/__threading_support:22:90: error: missing binary operator
> before token "("
>  #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) && (!defined(__has_include)
> || __has_include(<__external_threading>))
>
> ^
> In file included from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/algorithm:637:0,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/__string:55,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/string_view:166,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/include/string:464,
>  from /usr/local/google/home/aizatsky/src/llvm/projects/
> libcxx/src/locale.cpp:16:
>
>
> On Sun, Sep 11, 2016 at 2:55 PM Asiri Rathnayake via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: asiri
>> Date: Sun Sep 11 16:46:40 2016
>> New Revision: 281179
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281179=rev
>> Log:
>> [libcxx] Introduce an externally-threaded libc++ variant.
>>
>> This patch further decouples libc++ from pthread, allowing libc++ to be
>> built
>> against other threading systems. There are two main use cases:
>>
>> - Building libc++ against a thread library other than pthreads.
>>
>> - Building libc++ with an "external" thread API, allowing a separate
>> library to
>>   provide the implementation of that API.
>>
>> The two use cases are quite similar, the second one being sligtly more
>> de-coupled than the first. The cmake option LIBCXX_HAS_EXTERNAL_THREAD_API
>> enables both kinds of builds. One needs to place an <__external_threading>
>> header file containing an implementation of the "libc++ thread API"
>> declared
>> in the <__threading_support> header.
>>
>> For the second use case, the implementation of the libc++ thread API can
>> delegate to a custom "external" thread API where the implementation of
>> this
>> external API is provided in a seperate library. This mechanism allows
>> toolchain
>> vendors to distribute a build of libc++ with a custom
>> thread-porting-layer API
>> (which is the "external" API above), platform vendors (recipients of the
>> toolchain/libc++) are then required to provide their implementation of
>> this API
>> to be linked with (end-user) C++ programs.
>>
>> Note that the second use case still requires establishing the basic types
>> that
>> get passed between the external thread library and the libc++ library
>> (e.g. __libcpp_mutex_t). These cannot be opaque pointer types (libc++
>> sources
>> won't compile otherwise). It 

Re: [PATCH] D22494: [analyzer] Explain why analyzer report is not generated (fix for PR12421).

[Anton Yartsev via cfe-commits]

ayartsev updated this revision to Diff 71056.
ayartsev added a comment.

Updated the patch. Important change in Options.td was missing in the last patch 
+ indentation fixed.
Still Ok to commit?


https://reviews.llvm.org/D22494

Files:
  include/clang/Driver/Options.td
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/diagnostics/diag-cross-file-boundaries.c
  test/Analysis/diagnostics/diag-cross-file-boundaries.h

Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.h
@@ -0,0 +1,4 @@
+static void f() {
+  int *p = 0;
+  *p = 1;   // expected-warning{{Dereference of null pointer}}
+}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=html -o 
PR12421.html %s 2>&1 | FileCheck %s
+
+// Test for PR12421
+#include "diag-cross-file-boundaries.h"
+
+int main(){
+  f();
+  return 0;
+}
+
+// CHECK: warning: Path diagnostic report is not generated.
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -211,6 +211,12 @@
 const SourceManager  = D->path.front()->getLocation().getManager();
 SmallVector WorkList;
 WorkList.push_back(>path);
+SmallString<128> buf;
+llvm::raw_svector_ostream warning(buf);
+warning << "warning: Path diagnostic report is not generated. Current "
+<< "output format does not support diagnostics that cross file "
+<< "boundaries. Refer to --analyzer-output for valid output "
+<< "formats\n";
 
 while (!WorkList.empty()) {
   const PathPieces  = *WorkList.pop_back_val();
@@ -222,19 +228,25 @@
 
 if (FID.isInvalid()) {
   FID = SMgr.getFileID(L);
-} else if (SMgr.getFileID(L) != FID)
-  return; // FIXME: Emit a warning?
+} else if (SMgr.getFileID(L) != FID) {
+  llvm::errs() << warning.str();
+  return;
+}
 
 // Check the source ranges.
 ArrayRef Ranges = piece->getRanges();
 for (ArrayRef::iterator I = Ranges.begin(),
  E = Ranges.end(); I != E; ++I) {
   SourceLocation L = SMgr.getExpansionLoc(I->getBegin());
-  if (!L.isFileID() || SMgr.getFileID(L) != FID)
-return; // FIXME: Emit a warning?
+  if (!L.isFileID() || SMgr.getFileID(L) != FID) {
+llvm::errs() << warning.str();
+return;
+  }
   L = SMgr.getExpansionLoc(I->getEnd());
-  if (!L.isFileID() || SMgr.getFileID(L) != FID)
-return; // FIXME: Emit a warning?
+  if (!L.isFileID() || SMgr.getFileID(L) != FID) {
+llvm::errs() << warning.str();
+return;
+  }
 }
 
 if (const PathDiagnosticCallPiece *call =
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1969,7 +1969,8 @@
 def _all_warnings : Flag<["--"], "all-warnings">, Alias;
 def _analyze_auto : Flag<["--"], "analyze-auto">, Flags<[DriverOption]>;
 def _analyzer_no_default_checks : Flag<["--"], "analyzer-no-default-checks">, 
Flags<[DriverOption]>;
-def _analyzer_output : JoinedOrSeparate<["--"], "analyzer-output">, 
Flags<[DriverOption]>;
+def _analyzer_output : JoinedOrSeparate<["--"], "analyzer-output">, 
Flags<[DriverOption]>,
+  HelpText<"Static analyzer report output format 
(html|plist|plist-multi-file|plist-html|text).">;
 def _analyze : Flag<["--"], "analyze">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Run the static analyzer">;
 def _assemble : Flag<["--"], "assemble">, Alias;


Index: test/Analysis/diagnostics/diag-cross-file-boundaries.h
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.h
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.h
@@ -0,0 +1,4 @@
+static void f() {
+  int *p = 0;
+  *p = 1;   // expected-warning{{Dereference of null pointer}}
+}
Index: test/Analysis/diagnostics/diag-cross-file-boundaries.c
===
--- test/Analysis/diagnostics/diag-cross-file-boundaries.c
+++ test/Analysis/diagnostics/diag-cross-file-boundaries.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+// RUN: %clang_cc1 -analyze 

Re: [PATCH] D24065: [libc++] Use _LIBCPP_TYPE_VIS_ONLY with enum class

[Shoaib Meenai via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281264: config: Use _LIBCPP_TYPE_VIS_ONLY with enum class 
(authored by smeenai).

Changed prior to commit:
  https://reviews.llvm.org/D24065?vs=70087=71059#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24065

Files:
  libcxx/trunk/include/__config

Index: libcxx/trunk/include/__config
===
--- libcxx/trunk/include/__config
+++ libcxx/trunk/include/__config
@@ -716,7 +716,7 @@
 _LIBCPP_ALWAYS_INLINE operator int() const {return __v_;} \
 };
 #else  // _LIBCPP_HAS_NO_STRONG_ENUMS
-#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS x
+#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS_ONLY x
 #define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
 #endif  // _LIBCPP_HAS_NO_STRONG_ENUMS
 


Index: libcxx/trunk/include/__config
===
--- libcxx/trunk/include/__config
+++ libcxx/trunk/include/__config
@@ -716,7 +716,7 @@
 _LIBCPP_ALWAYS_INLINE operator int() const {return __v_;} \
 };
 #else  // _LIBCPP_HAS_NO_STRONG_ENUMS
-#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS x
+#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS_ONLY x
 #define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
 #endif  // _LIBCPP_HAS_NO_STRONG_ENUMS
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r281264 - config: Use _LIBCPP_TYPE_VIS_ONLY with enum class

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Sep 12 16:41:12 2016
New Revision: 281264

URL: http://llvm.org/viewvc/llvm-project?rev=281264=rev
Log:
config: Use _LIBCPP_TYPE_VIS_ONLY with enum class

An enum class has associated type info. In the Microsoft ABI, type info
is emitted in the COMDAT section and isn't exported, so clang rightfully
complains about __declspec(dllexport) being unused for an enum class.
On other platforms, we still want to export the type info.

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

Modified:
libcxx/trunk/include/__config

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=281264=281263=281264=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Mon Sep 12 16:41:12 2016
@@ -716,7 +716,7 @@ template  struct __static_asse
 _LIBCPP_ALWAYS_INLINE operator int() const {return __v_;} \
 };
 #else  // _LIBCPP_HAS_NO_STRONG_ENUMS
-#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS x
+#define _LIBCPP_DECLARE_STRONG_ENUM(x) enum class _LIBCPP_TYPE_VIS_ONLY x
 #define _LIBCPP_DECLARE_STRONG_ENUM_EPILOG(x)
 #endif  // _LIBCPP_HAS_NO_STRONG_ENUMS
 


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

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Eric Liu via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D24383#539942, @djasper wrote:

> Ok.. Thought some more and discussed with Manuel.
>
> I think we should do a partial solution for now. I still think addOrMerge is 
> harmful and it is always never the right thing to use. The codepaths that 
> currently implement some version of it, should likely reconsider.
>
> What I think we should implement is that some combinations of inserts and 
> replacements are accepted by "add". Specifically, imagine you have an 
> original text "aa" and a replacement that replaces it to "bb". Now, imagine 
> you have an additional insert of "c" at offsets 0, 1 or 2:
>
> - Offset 0: Not ambiguous, result should be "cbb".
> - Offset 1: Conflict, report error.
> - Offset 2: Not ambiguous, result should be "bbc"
>
>   So basically, inserts adjacent to replacements (including deletions) have a 
> well defined order and should be supported.
>
>   Two replacements still need a defined order and we should probably error 
> for now. I have some ideas on how to solve this. The most intuitive one might 
> be to have a pair functions "StringRef getReplacement(unsigned offset)"/"void 
> setReplacement(unsigned offset, StringRef text)", that control the 
> replacement at a given offset. A replacement object shouldn't have more than 
> one insert at any given offset. But again, I think that's a problem we can 
> get to later. I think most tools that currently would insert stuff at the 
> same location are actually wrong and we should continue to error.


If I understand correctly, `add` should support:

- adding non-conflict replacement as before.
- adding insertion that is adjacent to but not contained in an existing 
replacement with length>0 (deletion, actual replacement), and vice versa. The 
replacement should always change the original code instead of inserted text.

But why don't we want to support inserting twice at the same offset? It seems 
reasonable to me to assume that insertions can be ordered by the time they are 
added. And there are also reasonable use cases to insert at the same offset. 
For example, a fix or tool that generates parentheses for expressions can 
easily generate two '(' at the same offset, e.g. `A && B || C && D`  -> `((A && 
B) || (C && D))`. Of course, this makes it impossible to order the later 
insertion before existing insertions with `add`, but I don't think we want to 
support that since it doesn't seem to be a reasonable way to add replacements. 
I've seen tools that relied on the previous `set` implementation to achieve 
this (i.e. insert before existing insertions), but the code is very weird and 
can be easily refactored to work without this feature. But if we must support 
it, we might want to an `insertBefore` option (default=false) to `add` 
interface that makes add new insertion before the existing/conflicting 
insertion.

> Similarly, we never want to merge overlapping replacements, but we should 
> ensure that directly adjacent ones work, e.g. "aa" can be convert into "bc" 
> with two replacements (one for the first character, one for the second) that 
> are combined with add().


I think this is already true with the current implementation of `add`.


https://reviews.llvm.org/D24383



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

[PATCH] D24472: [Sema] Support lax conversions for compound assignments

[Bruno Cardoso Lopes via cfe-commits]

bruno created this revision.
bruno added a reviewer: rnk.
bruno added subscribers: cfe-commits, ahatanak.

Support lax conversions on compound assignment expressions like:

  typedef __attribute__((vector_size(8))) double float64x1_t;
  typedef __attribute__((vector_size(16))) double float64x2_t;
  float64x1_t vget_low_f64(float64x2_t __p0);

  double c = 3.0;
  float64x2_t v = {0.0, 1.0};
  c += vget_low_f64(v);

This restores one more valid behavior pre r266366, and is a incremental
follow up from work committed in r274646.

While here, make the check more strict, add FIXMEs, clean up variable
names to match what they can actually be and update testcase to reflect
that. We now reject:

  typedef float float2 __attribute__ ((vector_size (8)));
  double d;
  f2 += d;

which doesn't fit as a direct bitcast anyway.

https://reviews.llvm.org/D24472

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/vector-cast.c

Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   float64x2_t v = {0.0, 1.0};
-  f2 += d;
+  // FIXME: These diagnostics are inaccurate: should complain that 'double' to 
vector 'float2' involves truncation
+  f2 += d; // expected-error {{cannot convert between vector values of 
different size ('float2' (vector of 2 'float' values) and 'double')}}
+  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
   a = 3.0 + vget_low_f64(v);
   b = vget_low_f64(v) + 3.0;
   c = vget_low_f64(v);
-  // LAX conversions within compound assignments are not supported.
-  // FIXME: This diagnostic is inaccurate.
-  d += f2; // expected-error {{cannot convert between vector values of 
different size}}
-  c -= vget_low_f64(v); // expected-error {{cannot convert between vector 
values of different size}}
+  c -= vget_low_f64(v);
   // LAX conversions between scalar and vector types require same size and one 
element sized vectors.
   d = f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
   d = d + f2; // expected-error {{assigning to 'double' from incompatible type 
'float2'}}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8045,6 +8045,7 @@
 
   // If there's an ext-vector type and a scalar, try to convert the scalar to
   // the vector element type and splat.
+  // FIXME: this should also work for regular vector types as supported in GCC.
   if (!RHSVecType && isa(LHSVecType)) {
 if (!tryVectorConvertAndSplat(*this, , RHSType,
   LHSVecType->getElementType(), LHSType))
@@ -8057,16 +8058,31 @@
   return RHSType;
   }
 
-  // If we're allowing lax vector conversions, only the total (data) size needs
-  // to be the same. If one of the types is scalar, the result is always the
-  // vector type. Don't allow this if the scalar operand is an lvalue.
+  // FIXME: The code below also handles convertion between vectors and
+  // non-scalars, we should break this down into fine grained specific checks
+  // and emit proper diagnostics.
   QualType VecType = LHSVecType ? LHSType : RHSType;
-  QualType ScalarType = LHSVecType ? RHSType : LHSType;
-  ExprResult *ScalarExpr = LHSVecType ?  : 
-  if (isLaxVectorConversion(ScalarType, VecType) &&
-  !ScalarExpr->get()->isLValue()) {
-*ScalarExpr = ImpCastExprToType(ScalarExpr->get(), VecType, CK_BitCast);
-return VecType;
+  const VectorType *VT = LHSVecType ? LHSVecType : RHSVecType;
+  QualType OtherType = LHSVecType ? RHSType : LHSType;
+  ExprResult *OtherExpr = LHSVecType ?  : 
+  if (isLaxVectorConversion(OtherType, VecType)) {
+// If we're allowing lax vector conversions, only the total (data) size
+// needs to be the same. For non compound assignment, if one of the types 
is
+// scalar, the result is always the vector type.
+if (!IsCompAssign) {
+  *OtherExpr = ImpCastExprToType(OtherExpr->get(), VecType, CK_BitCast);
+  return VecType;
+// In a compound assignment, lhs += rhs, 'lhs' is a lvalue src, forbidding
+// any implicit cast. Here, the 'rhs' should be implicit casted to 'lhs'
+// type. Note that this is already done by non-compound assignments in
+// CheckAssignmentConstraints. If it's a scalar type, only biscast for
+// <1 x T> -> T.
+} else if (OtherType->isExtVectorType() ||
+   (OtherType->isScalarType() && VT->getNumElements() == 1)) {
+  ExprResult *RHSExpr = 
+  *RHSExpr = ImpCastExprToType(RHSExpr->get(), LHSType, CK_BitCast);
+  return LHSType;
+}
   }
 
   // Okay, the expression is invalid.


Index: test/Sema/vector-cast.c
===
--- test/Sema/vector-cast.c
+++ test/Sema/vector-cast.c
@@ -53,14 +53,13 @@
   float2 f2;
   double d, a, b, c;
   

[libcxx] r281250 - config: Fix typo in comment

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Sep 12 15:14:44 2016
New Revision: 281250

URL: http://llvm.org/viewvc/llvm-project?rev=281250=rev
Log:
config: Fix typo in comment

Testing commit access. NFC.

Modified:
libcxx/trunk/include/__config

Modified: libcxx/trunk/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=281250=281249=281250=diff
==
--- libcxx/trunk/include/__config (original)
+++ libcxx/trunk/include/__config Mon Sep 12 15:14:44 2016
@@ -34,7 +34,7 @@
 #endif
 
 #if defined(_LIBCPP_ABI_UNSTABLE) || _LIBCPP_ABI_VERSION >= 2
-// Change short string represention so that string data starts at offset 0,
+// Change short string representation so that string data starts at offset 0,
 // improving its alignment in some cases.
 #define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 // Fix deque iterator type in order to support incomplete types.


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

Re: [libcxx] r281179 - [libcxx] Introduce an externally-threaded libc++ variant.

[Mike Aizatsky via cfe-commits]

Asiri,

The boot strap build has started to fail for. Your change seems to be
related, right?

[14/22] Building CXX object
projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o
FAILED: /usr/bin/g++   -D_DEBUG -D_GNU_SOURCE
-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/libcxx/lib
-I/usr/local/google/home/aizatsky/
src/llvm/projects/libcxx/lib -Iinclude
-I/usr/local/google/home/aizatsky/src/llvm/include
-I/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include -fPIC
-fvisibility-inlines-hidden -Wall -W -Wno-unuse
d-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers
 -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
-Wno-comment -std=c++11 -ffunction-sections -fdata-sections -O3 -UN
DEBUG -DLIBCXX_BUILDING_LIBCXXABI -std=c++11 -nostdinc++ -Wall -Wextra -W
-Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type
-Wno-attributes -Wno-literal-suffix -Wno-c++14-compat -Wno-erro
r -fPIC -MMD -MT
projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o -MF
projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o.d -o
projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/
src/locale.cpp.o -c
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/src/locale.cpp
In file included from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/__mutex_base:17:0,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/mutex:189,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/__locale:18,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/locale:182,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/src/locale.cpp:17:
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/__threading_support:22:90:
error: missing binary operator before token "("
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) && (!defined(__has_include)
|| __has_include(<__external_threading>))

  ^
In file included from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/algorithm:637:0,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/__string:55,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/string_view:166,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include/string:464,
 from
/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/src/locale.cpp:16:


On Sun, Sep 11, 2016 at 2:55 PM Asiri Rathnayake via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: asiri
> Date: Sun Sep 11 16:46:40 2016
> New Revision: 281179
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281179=rev
> Log:
> [libcxx] Introduce an externally-threaded libc++ variant.
>
> This patch further decouples libc++ from pthread, allowing libc++ to be
> built
> against other threading systems. There are two main use cases:
>
> - Building libc++ against a thread library other than pthreads.
>
> - Building libc++ with an "external" thread API, allowing a separate
> library to
>   provide the implementation of that API.
>
> The two use cases are quite similar, the second one being sligtly more
> de-coupled than the first. The cmake option LIBCXX_HAS_EXTERNAL_THREAD_API
> enables both kinds of builds. One needs to place an <__external_threading>
> header file containing an implementation of the "libc++ thread API"
> declared
> in the <__threading_support> header.
>
> For the second use case, the implementation of the libc++ thread API can
> delegate to a custom "external" thread API where the implementation of this
> external API is provided in a seperate library. This mechanism allows
> toolchain
> vendors to distribute a build of libc++ with a custom thread-porting-layer
> API
> (which is the "external" API above), platform vendors (recipients of the
> toolchain/libc++) are then required to provide their implementation of
> this API
> to be linked with (end-user) C++ programs.
>
> Note that the second use case still requires establishing the basic types
> that
> get passed between the external thread library and the libc++ library
> (e.g. __libcpp_mutex_t). These cannot be opaque pointer types (libc++
> sources
> won't compile otherwise). It should also be noted that the second use case
> can
> have a slight performance penalty; as all the thread constructs need to
> cross a
> library boundary through an additional function call.
>
> When the header <__external_threading> is omitted, libc++ is built with the
> "libc++ thread API" (declared in <__threading_support>) as the "external"
> thread
> API (basic types are pthread based). An implementation (pthread based) of
> this
> API is provided in test/support/external_threads.cpp, which is built into a
> separate DSO and linked in when running the libc++ test suite. A 

Re: [PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

[Nico Weber via cfe-commits]

thakis updated this revision to Diff 71050.
thakis added a comment.

On conflict, keep first uuid in ast instead of having different uuids on 
different redeclarations.


https://reviews.llvm.org/D24469

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCXX/ms-uuid.cpp

Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s
+
+typedef struct _GUID {
+  unsigned long Data1;
+  unsigned short Data2;
+  unsigned short Data3;
+  unsigned char Data4[8];
+} GUID;
+
+namespace {
+// cl.exe's behavior with merging uuid attributes is a bit erratic:
+// * In []-style attributes, a single [] list must not list a duplicate uuid
+//   (even if it's the same uuid), and only a single declaration of a class
+//   must have a uuid else the compiler errors out (even if two declarations of
+//   a class have the same uuid).
+// * For __declspec(uuid(...)), it's ok if several declarations of a class have
+//   an uuid, as long as it's the same uuid each time.  If uuids on declarations
+//   don't match, the compiler errors out.
+// * If there are several __declspec(uuid(...))s on one declaration, the
+//   compiler only warns about this and uses the last uuid.  It even warns if
+//   the uuids are the same.
+
+// clang-cl implements the following simpler (but largely compatible) behavior
+// instead:
+// * [] and __declspec uuids have the same behavior.
+// * If there are several uuids on a a class (no matter if on the same decl or
+//   on several decls), it is an error if they don't match.
+// * Having several uuids that match is ok.
+
+// Both cl and clang-cl accept this:
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 2{{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("11A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("22A0---C000-0049")) C2 {};
+
+// clang-cl accepts this, but cl errors out:
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3 {};
+
+// Both cl and clang-cl error out on this (but for different reasons):
+// expected-note@+1 2{{previous uuid specified here}}
+[uuid("00A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}}
+[uuid("11A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}}
+[uuid("22A0---C000-0049")] class C4 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049"))
+// expected-error@+1 {{uiid does not match previous declaration}}
+  __declspec(uuid("11A0---C000-0049")) C5;
+
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049"),
+// expected-error@+1 {{uiid does not match previous declaration}}
+ uuid("11A0---C000-0049")] class C6;
+
+// cl doesn't diagnose having one uuid each as []-style attributes and as
+// __declspec, even if the uuids differ.  clang-cl errors if they differ.
+[uuid("00A0---C000-0049")]
+class __declspec(uuid("00A0---C000-0049")) C7;
+
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049")]
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("11A0---C000-0049")) C8;
+
+
+// cl warns on this, but clang-cl is fine with it (which is consistent with
+// e.g. specifying __multiple_inheritance several times, which cl accepts
+// without warning too).
+class __declspec(uuid("00A0---C000-0049"))
+  __declspec(uuid("00A0---C000-0049")) C9;
+
+// cl errors out on this, but clang-cl is fine with it (to be consistent with
+// the previous case).
+[uuid("00A0---C000-0049"),
+ uuid("00A0---C000-0049")] class C10;
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,6 +4604,19 @@
 // Microsoft 

Re: [libcxx] r281179 - [libcxx] Introduce an externally-threaded libc++ variant.

[Asiri Rathnayake via cfe-commits]

OK, I'm able to reproduce locally. Looks like gcc (4.8.3 at least) does not
like my patch.

Working on a fix...

On Mon, Sep 12, 2016 at 9:43 PM, Asiri Rathnayake <
asiri.rathnay...@gmail.com> wrote:

> Hi Mike,
>
> That does look related to my change. Is this an internal build? I was
> looking out for the bots on llvm.org but didn't spot any issues (yet).
>
> Will try to reproduce locally. Would it be OK if I find a fix tomorrow?
> (night time here in the UK), can revert if this is blocking you.
>
> Cheers,
>
> / Asiri
>
>
> On Mon, Sep 12, 2016 at 9:25 PM, Mike Aizatsky via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Asiri,
>>
>> The boot strap build has started to fail for. Your change seems to be
>> related, right?
>>
>> [14/22] Building CXX object projects/libcxx/lib/CMakeFiles
>> /cxx_objects.dir/__/src/locale.cpp.o
>> FAILED: /usr/bin/g++   -D_DEBUG -D_GNU_SOURCE
>> -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D__STDC_CONSTANT_MACROS
>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/libcxx/lib
>> -I/usr/local/google/home/aizatsky/
>> src/llvm/projects/libcxx/lib -Iinclude 
>> -I/usr/local/google/home/aizatsky/src/llvm/include
>> -I/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include -fPIC
>> -fvisibility-inlines-hidden -Wall -W -Wno-unuse
>> d-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers
>>  -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
>> -Wno-comment -std=c++11 -ffunction-sections -fdata-sections -O3 -UN
>> DEBUG -DLIBCXX_BUILDING_LIBCXXABI -std=c++11 -nostdinc++ -Wall -Wextra -W
>> -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type
>> -Wno-attributes -Wno-literal-suffix -Wno-c++14-compat -Wno-erro
>> r -fPIC -MMD -MT 
>> projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o
>> -MF projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/src/locale.cpp.o.d
>> -o projects/libcxx/lib/CMakeFiles/cxx_objects.dir/__/
>> src/locale.cpp.o -c /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/src/locale.cpp
>> In file included from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/__mutex_base:17:0,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/mutex:189,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/__locale:18,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/locale:182,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/src/locale.cpp:17:
>> /usr/local/google/home/aizatsky/src/llvm/projects/libcxx/
>> include/__threading_support:22:90: error: missing binary operator before
>> token "("
>>  #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) &&
>> (!defined(__has_include) || __has_include(<__external_threading>))
>>
>> ^
>> In file included from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/algorithm:637:0,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/__string:55,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/string_view:166,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/include/string:464,
>>  from /usr/local/google/home/aizatsk
>> y/src/llvm/projects/libcxx/src/locale.cpp:16:
>>
>>
>> On Sun, Sep 11, 2016 at 2:55 PM Asiri Rathnayake via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: asiri
>>> Date: Sun Sep 11 16:46:40 2016
>>> New Revision: 281179
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=281179=rev
>>> Log:
>>> [libcxx] Introduce an externally-threaded libc++ variant.
>>>
>>> This patch further decouples libc++ from pthread, allowing libc++ to be
>>> built
>>> against other threading systems. There are two main use cases:
>>>
>>> - Building libc++ against a thread library other than pthreads.
>>>
>>> - Building libc++ with an "external" thread API, allowing a separate
>>> library to
>>>   provide the implementation of that API.
>>>
>>> The two use cases are quite similar, the second one being sligtly more
>>> de-coupled than the first. The cmake option
>>> LIBCXX_HAS_EXTERNAL_THREAD_API
>>> enables both kinds of builds. One needs to place an
>>> <__external_threading>
>>> header file containing an implementation of the "libc++ thread API"
>>> declared
>>> in the <__threading_support> header.
>>>
>>> For the second use case, the implementation of the libc++ thread API can
>>> delegate to a custom "external" thread API where the implementation of
>>> this
>>> external API is provided in a seperate library. This mechanism allows
>>> toolchain
>>> vendors to distribute a build of libc++ with a custom
>>> thread-porting-layer API
>>> (which is the "external" API above), platform vendors (recipients of the
>>> toolchain/libc++) are then required to provide their implementation of
>>> this 

Re: [libcxx] r281179 - [libcxx] Introduce an externally-threaded libc++ variant.

[Asiri Rathnayake via cfe-commits]

Hi Mike,

Can you check if https://reviews.llvm.org/D24475 fixes the issue for you?
This looks like a bug in the gcc pre-processor (in older versions of gcc).

I'm a bit hesitant because my gcc (4.8.3) bumps into a different issue
(crash!) as well, but that seem to have nothing to do with my patch (can
reproduce without my patch...).

Cheers,

/ Asiri

On Mon, Sep 12, 2016 at 9:49 PM, Asiri Rathnayake <
asiri.rathnay...@gmail.com> wrote:

> OK, I'm able to reproduce locally. Looks like gcc (4.8.3 at least) does
> not like my patch.
>
> Working on a fix...
>
> On Mon, Sep 12, 2016 at 9:43 PM, Asiri Rathnayake <
> asiri.rathnay...@gmail.com> wrote:
>
>> Hi Mike,
>>
>> That does look related to my change. Is this an internal build? I was
>> looking out for the bots on llvm.org but didn't spot any issues (yet).
>>
>> Will try to reproduce locally. Would it be OK if I find a fix tomorrow?
>> (night time here in the UK), can revert if this is blocking you.
>>
>> Cheers,
>>
>> / Asiri
>>
>>
>> On Mon, Sep 12, 2016 at 9:25 PM, Mike Aizatsky via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Asiri,
>>>
>>> The boot strap build has started to fail for. Your change seems to be
>>> related, right?
>>>
>>> [14/22] Building CXX object projects/libcxx/lib/CMakeFiles
>>> /cxx_objects.dir/__/src/locale.cpp.o
>>> FAILED: /usr/bin/g++   -D_DEBUG -D_GNU_SOURCE
>>> -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D__STDC_CONSTANT_MACROS
>>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/libcxx/lib
>>> -I/usr/local/google/home/aizatsky/
>>> src/llvm/projects/libcxx/lib -Iinclude 
>>> -I/usr/local/google/home/aizatsky/src/llvm/include
>>> -I/usr/local/google/home/aizatsky/src/llvm/projects/libcxx/include
>>> -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unuse
>>> d-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers
>>>  -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
>>> -Wno-comment -std=c++11 -ffunction-sections -fdata-sections -O3 -UN
>>> DEBUG -DLIBCXX_BUILDING_LIBCXXABI -std=c++11 -nostdinc++ -Wall -Wextra
>>> -W -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type
>>> -Wno-attributes -Wno-literal-suffix -Wno-c++14-compat -Wno-erro
>>> r -fPIC -MMD -MT projects/libcxx/lib/CMakeFiles
>>> /cxx_objects.dir/__/src/locale.cpp.o -MF projects/libcxx/lib/CMakeFiles
>>> /cxx_objects.dir/__/src/locale.cpp.o.d -o projects/libcxx/lib/CMakeFiles
>>> /cxx_objects.dir/__/
>>> src/locale.cpp.o -c /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/src/locale.cpp
>>> In file included from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/__mutex_base:17:0,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/mutex:189,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/__locale:18,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/locale:182,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/src/locale.cpp:17:
>>> /usr/local/google/home/aizatsky/src/llvm/projects/libcxx/inc
>>> lude/__threading_support:22:90: error: missing binary operator before
>>> token "("
>>>  #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL) &&
>>> (!defined(__has_include) || __has_include(<__external_threading>))
>>>
>>>   ^
>>> In file included from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/algorithm:637:0,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/__string:55,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/string_view:166,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/include/string:464,
>>>  from /usr/local/google/home/aizatsk
>>> y/src/llvm/projects/libcxx/src/locale.cpp:16:
>>>
>>>
>>> On Sun, Sep 11, 2016 at 2:55 PM Asiri Rathnayake via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: asiri
 Date: Sun Sep 11 16:46:40 2016
 New Revision: 281179

 URL: http://llvm.org/viewvc/llvm-project?rev=281179=rev
 Log:
 [libcxx] Introduce an externally-threaded libc++ variant.

 This patch further decouples libc++ from pthread, allowing libc++ to be
 built
 against other threading systems. There are two main use cases:

 - Building libc++ against a thread library other than pthreads.

 - Building libc++ with an "external" thread API, allowing a separate
 library to
   provide the implementation of that API.

 The two use cases are quite similar, the second one being sligtly more
 de-coupled than the first. The cmake option
 LIBCXX_HAS_EXTERNAL_THREAD_API
 enables both kinds of builds. One needs to place an
 <__external_threading>
 header file containing an implementation of the "libc++ thread 

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Sam Shepperd via cfe-commits]

phabricss added a comment.

On 09/12/2016 01:26 PM, Nick Lewycky wrote:

> Firstly, I thought glibc had applied a patch to fix this bug? As in, the 
> error is correct and glibc fixed their bug?


I can confirm that the bug still exists in glibc 2.24 and HEAD from glibc git.  
Also it appears that the issue on the llvm mailing list was just dropped 
without any resolution:

r255371 - Error on redeclaring with a conflicting asm label 


  ../include/sys/stat.h:16:15: error: cannot apply asm label to function after 
its first use
  hidden_proto (__fxstat)
  ~~^
  ./../include/libc-symbols.h:420:19: note: expanded from macro 'hidden_proto'
__hidden_proto (name, , __GI_##name, ##attrs)
^
  ./../include/libc-symbols.h:424:33: note: expanded from macro '__hidden_proto'
extern thread __typeof (name) name __asm__ (__hidden_asmname (#internal)) \

As far as your review of the patch by weimingz, that is beyond my skill level 
so I will let him reply.  Thanks!


https://reviews.llvm.org/D16171



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

r281259 - Add a couple of test files missed in r281258.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 16:07:09 2016
New Revision: 281259

URL: http://llvm.org/viewvc/llvm-project?rev=281259=rev
Log:
Add a couple of test files missed in r281258.

Added:
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h

Added: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h?rev=281259=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h Mon Sep 
12 16:07:09 2016
@@ -0,0 +1,9 @@
+namespace CrossModuleMerge {
+  template struct A;
+  template struct B;
+
+  template struct A {};
+  template struct B : A {};
+  template inline auto C(T) {}
+}
+

Added: cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h?rev=281259=auto
==
--- cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/d.h Mon Sep 
12 16:07:09 2016
@@ -0,0 +1 @@
+// d.h: empty


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

r281258 - [modules] When we merge two definitions of a function, mark the retained

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 12 16:06:40 2016
New Revision: 281258

URL: http://llvm.org/viewvc/llvm-project?rev=281258=rev
Log:
[modules] When we merge two definitions of a function, mark the retained
definition as visible in the discarded definition's module, as we do for
other kinds of definition.

Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h

cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/module.modulemap
cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=281258=281257=281258=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Mon Sep 12 16:06:40 2016
@@ -1420,6 +1420,10 @@ public:
   /// \brief Make the names within this set of hidden names visible.
   void makeNamesVisible(const HiddenNames , Module *Owner);
 
+  /// \brief Note that MergedDef is a redefinition of the canonical definition
+  /// Def, so Def should be visible whenever MergedDef is.
+  void mergeDefinitionVisibility(NamedDecl *Def, NamedDecl *MergedDef);
+
   /// \brief Take the AST callbacks listener.
   std::unique_ptr takeListener() {
 return std::move(Listener);

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=281258=281257=281258=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Sep 12 16:06:40 2016
@@ -3469,6 +3469,31 @@ void ASTReader::makeModuleVisible(Module
   }
 }
 
+/// We've merged the definition \p MergedDef into the existing definition
+/// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made
+/// visible.
+void ASTReader::mergeDefinitionVisibility(NamedDecl *Def,
+  NamedDecl *MergedDef) {
+  // FIXME: This doesn't correctly handle the case where MergedDef is visible
+  // in modules other than its owning module. We should instead give the
+  // ASTContext a list of merged definitions for Def.
+  if (Def->isHidden()) {
+// If MergedDef is visible or becomes visible, make the definition visible.
+if (!MergedDef->isHidden())
+  Def->Hidden = false;
+else if (getContext().getLangOpts().ModulesLocalVisibility) {
+  getContext().mergeDefinitionIntoModule(
+  Def, MergedDef->getImportedOwningModule(),
+  /*NotifyListeners*/ false);
+  PendingMergedDefinitionsToDeduplicate.insert(Def);
+} else {
+  auto SubmoduleID = MergedDef->getOwningModuleID();
+  assert(SubmoduleID && "hidden definition in no module");
+  HiddenNamesMap[getSubmodule(SubmoduleID)].push_back(Def);
+}
+  }
+}
+
 bool ASTReader::loadGlobalIndex() {
   if (GlobalIndex)
 return false;
@@ -8574,8 +8599,11 @@ void ASTReader::finishPendingActions() {
 if (FunctionDecl *FD = dyn_cast(PB->first)) {
   // FIXME: Check for =delete/=default?
   // FIXME: Complain about ODR violations here?
-  if (!getContext().getLangOpts().Modules || !FD->hasBody())
+  const FunctionDecl *Defn = nullptr;
+  if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
 FD->setLazyBody(PB->second);
+  else
+mergeDefinitionVisibility(const_cast(Defn), FD);
   continue;
 }
 

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=281258=281257=281258=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Sep 12 16:06:40 2016
@@ -383,27 +383,6 @@ namespace clang {
 void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
 void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D);
 void VisitOMPCapturedExprDecl(OMPCapturedExprDecl *D);
-
-/// We've merged the definition \p MergedDef into the existing definition
-/// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is 
made
-/// visible.
-void mergeDefinitionVisibility(NamedDecl *Def, NamedDecl *MergedDef) {
-  if (Def->isHidden()) {
-// If MergedDef is visible or becomes visible, make the definition 
visible.
-if (!MergedDef->isHidden())
-  Def->Hidden = false;
-else if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
-  

Re: [PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

[Nico Weber via cfe-commits]

thakis updated this revision to Diff 71049.
thakis added a comment.

Remove local XXXs I've since addressed.


https://reviews.llvm.org/D24469

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCXX/ms-uuid.cpp

Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s
+
+typedef struct _GUID {
+  unsigned long Data1;
+  unsigned short Data2;
+  unsigned short Data3;
+  unsigned char Data4[8];
+} GUID;
+
+namespace {
+// cl.exe's behavior with merging uuid attributes is a bit erratic:
+// * In []-style attributes, a single [] list must not list a duplicate uuid
+//   (even if it's the same uuid), and only a single declaration of a class
+//   must have a uuid else the compiler errors out (even if two declarations of
+//   a class have the same uuid).
+// * For __declspec(uuid(...)), it's ok if several declarations of a class have
+//   an uuid, as long as it's the same uuid each time.  If uuids on declarations
+//   don't match, the compiler errors out.
+// * If there are several __declspec(uuid(...))s on one declaration, the
+//   compiler only warns about this and uses the last uuid.  It even warns if
+//   the uuids are the same.
+
+// clang-cl implements the following simpler (but largely compatible) behavior
+// instead:
+// * [] and __declspec uuids have the same behavior.
+// * If there are several uuids on a a class (no matter if on the same decl or
+//   on several decls), it is an error if they don't match.
+// * Having several uuids that match is ok.
+
+// Both cl and clang-cl accept this:
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1;
+class __declspec(uuid("00A0---C000-0049")) C1 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("11A0---C000-0049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("22A0---C000-0049")) C2 {};
+
+// clang-cl accepts this, but cl errors out:
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3;
+[uuid("00A0---C000-0049")] class C3 {};
+
+// Both cl and clang-cl error out on this (but for different reasons):
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+[uuid("11A0---C000-0049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}}
+[uuid("22A0---C000-0049")] class C4 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("00A0---C000-0049"))
+// expected-error@+1 {{uiid does not match previous declaration}}
+  __declspec(uuid("11A0---C000-0049")) C5;
+
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049"),
+// expected-error@+1 {{uiid does not match previous declaration}}
+ uuid("11A0---C000-0049")] class C6;
+
+// cl doesn't diagnose having one uuid each as []-style attributes and as
+// __declspec, even if the uuids differ.  clang-cl errors if they differ.
+[uuid("00A0---C000-0049")]
+class __declspec(uuid("00A0---C000-0049")) C7;
+
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("00A0---C000-0049")]
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("11A0---C000-0049")) C8;
+
+
+// cl warns on this, but clang-cl is fine with it (which is consistent with
+// e.g. specifying __multiple_inheritance several times, which cl accepts
+// without warning too).
+class __declspec(uuid("00A0---C000-0049"))
+  __declspec(uuid("00A0---C000-0049")) C9;
+
+// cl errors out on this, but clang-cl is fine with it (to be consistent with
+// the previous case).
+[uuid("00A0---C000-0049"),
+ uuid("00A0---C000-0049")] class C10;
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ 

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Daniel Jasper via cfe-commits]

djasper added a comment.

The problem is that it is implicit behavior, that's not easy to understand. 
What's worse is that the behavior of add and merge would fundamentally be 
reverse. If you add two inserts at the same location, the first one would come 
first. If you merge them, the second one would come first.

And you parentheses example is actually a good one. The tool you would write 
for that would insert the parentheses pair-wise, e.g. in an ASTMatchFinder 
callback. Without extra effort, the opening parentheses and the closing 
parentheses would be inserted in the same order, which is wrong. Doesn't matter 
as they are identical, but assume you'd want to add () and [].. You'd likely 
end up with "([...)]".

We should definitely provide ways to support such use cases, but I think the 
default behavior should for now be to report an error.


https://reviews.llvm.org/D24383



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

Re: [PATCH] D24467: Fix an error after D21678

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());

It wasn't clear to me why we have to call DefaultFunctionArrayLvalueConversion 
instead of UsualUnaryConversions. Is it possible to come up with a test case 
that would fail without this change, and if it is, can you add it to vecshift.c?


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();

We no longer error out when LHS is not a vector, so it should mention that 
either the LHS or the RHS might not be a vector.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8752
@@ -8747,3 +8751,3 @@
 
   // OpenCL v1.1 s6.3.j says that the operands need to be integers.
   if (!LHSEleType->isIntegerType()) {

This comment should be updated since other vector types (e.g., gcc's vector 
type) are handled by this function too.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8774
@@ +8773,3 @@
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);

Is it correct to always create an ExtVectorType here? What if the RHS is a 
GenericVector?


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();

Is it possible to split this out to another patch? The first patch would fix 
handling of (scalar << vector) and the second patch would make clang reject 
vector shifts if element sizes of the LHS and RHS are different. It's not clear 
whether it's correct to reject the latter case, so perhaps you can discuss it 
in a separate patch? 


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

When the LHS is a scalar, we check the type of the LHS and the element type of 
the RHS, and if necessary, cast the LHS to the element type of the RHS. What's 
the reason we don't do the same here?


Repository:
  rL LLVM

https://reviews.llvm.org/D24467



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

Re: [PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

[Reid Kleckner via cfe-commits]

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

lgtm



Comment at: lib/Parse/ParseDecl.cpp:1456
@@ -1451,1 +1455,3 @@
+  TypeAttrTail = AL;
+  TypeAttrTail->setNext(nullptr);
 

mmm hand rolled singly linked list code. Feels like lisp.


https://reviews.llvm.org/D24469



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

Re: [PATCH] D23852: [SemaObjC] Fix crash while parsing type arguments and protocols

[Bruno Cardoso Lopes via cfe-commits]

bruno added a comment.

Ping!


https://reviews.llvm.org/D23852



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

Re: r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

[Richard Smith via cfe-commits]

On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Does that actually need a virtual dtor? The type erasure handling in
> shared_ptr would handle it so long as each instance is constructed with the
> derived type (looks like it probably is - make_shared used consistently) &
> we should get a warning (or could make the base dtor protected non-virtual
> to ensure an error) if we miss that?
>

We get warnings from GCC regardless, and that broke a few bots :( I'd like
to switch this back to unique_ptr at some point (once I've figured out why
MSVC rejected it) and at that point we'll need the virtual destructor.

But I can understand that it's perhaps nice to do even if we could thread
> that delicate needle. Just curious.
>
> On Sun, Sep 11, 2016 at 11:59 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Mon Sep 12 01:51:11 2016
>> New Revision: 281198
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281198=rev
>> Log:
>> Add virtual destructor (necessary due to the switch to shared_ptr).
>>
>> Modified:
>> cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>>
>> Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/
>> ClangDiagnosticsEmitter.cpp?rev=281198=281197=281198=diff
>> 
>> ==
>> --- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
>> +++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12
>> 01:51:11 2016
>> @@ -911,6 +911,7 @@ namespace {
>>  struct DiagText {
>>struct Piece {
>>  virtual void print(std::vector ) = 0;
>> +virtual ~Piece() {}
>>};
>>struct TextPiece : Piece {
>>  StringRef Role;
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit

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

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

Using lodato's proposed interface. This patch:

- Accepts an arbitrary number of commits as arguments. Validation will be done 
in main(), such that two commits are valid only when running in --diff mode.
- Allows diffing two commits against each other in addition to diffing against 
the working directory (still using git diff-tree). The previous version didn't 
really work with merge-commits anyways.
- Runs clang-format against the file in the working directory (if a single 
commit is passed), or against the second commit.


https://reviews.llvm.org/D24319

Files:
  tools/clang-format/git-clang-format

Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
 usage = 'git clang-format [OPTIONS] [] [--] [...]'
 
 desc = '''
-Run clang-format on all lines that differ between the working directory
-and , which defaults to HEAD.  Changes are only applied to the working
-directory.
+Run clang-format on all lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit.  Changes are only applied to the working directory.
 
 The following git-config settings set the default of the corresponding option:
   clangFormat.binary
@@ -120,8 +120,14 @@
   opts.verbose -= opts.quiet
   del opts.quiet
 
-  commit, files = interpret_args(opts.args, dash_dash, opts.commit)
-  changed_lines = compute_diff_and_extract_lines(commit, files)
+  commits, files = interpret_args(opts.args, dash_dash, opts.commit)
+  if not opts.diff:
+if len(commits) > 1:
+  die('at most one commit allowed; %d given' % len(commits))
+  else:
+if len(commits) > 2:
+  die('at most two commits allowed; %d given' % len(commits))
+  changed_lines = compute_diff_and_extract_lines(commits, files)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,10 +147,17 @@
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.
   cd_to_toplevel()
-  old_tree = create_tree_from_workdir(changed_lines)
-  new_tree = run_clang_format_and_save_to_tree(changed_lines,
-   binary=opts.binary,
-   style=opts.style)
+  if len(commits) > 1:
+old_tree = create_tree_from_commit(commits[1], changed_lines)
+new_tree = run_clang_format_and_save_to_tree(changed_lines,
+ revision=commits[1],
+ binary=opts.binary,
+ style=opts.style)
+  else:
+old_tree = create_tree_from_workdir(changed_lines)
+new_tree = run_clang_format_and_save_to_tree(changed_lines,
+ binary=opts.binary,
+ style=opts.style)
   if opts.verbose >= 1:
 print 'old tree:', old_tree
 print 'new tree:', new_tree
@@ -181,40 +194,42 @@
 
 
 def interpret_args(args, dash_dash, default_commit):
-  """Interpret `args` as "[commit] [--] [files...]" and return (commit, files).
+  """Interpret `args` as "[commits...] [--] [files...]" and return (commits,
+  files).
 
   It is assumed that "--" and everything that follows has been removed from
   args and placed in `dash_dash`.
 
-  If "--" is present (i.e., `dash_dash` is non-empty), the argument to its
-  left (if present) is taken as commit.  Otherwise, the first argument is
-  checked if it is a commit or a file.  If commit is not given,
-  `default_commit` is used."""
+  If "--" is present (i.e., `dash_dash` is non-empty), the arguments to its
+  left (if present) are taken as commits.  Otherwise, the arguments are checked
+  from left to right if they are commits or files.  If commits are not given,
+  a list with `default_commit` is used."""
   if dash_dash:
 if len(args) == 0:
-  commit = default_commit
-elif len(args) > 1:
-  die('at most one commit allowed; %d given' % len(args))
+  commits = [default_commit]
 else:
-  commit = args[0]
-object_type = get_object_type(commit)
-if object_type not in ('commit', 'tag'):
-  if object_type is None:
-die("'%s' is not a commit" % commit)
-  else:
-die("'%s' is a %s, but a commit was expected" % (commit, object_type))
+  commits = args
+for commit in commits:
+  object_type = get_object_type(commit)
+  if object_type not in ('commit', 'tag'):
+if object_type is None:
+  die("'%s' is not a commit" % commit)
+else:
+  die("'%s' is a %s, but a commit was expected" % (commit, object_type))
 files = dash_dash[1:]
   elif args:
-if 

Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit

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

lhchavez marked 3 inline comments as done.


Comment at: cfe/trunk/tools/clang-format/git-clang-format:93
@@ -92,1 +92,3 @@
  help='default commit to use if none is specified'),
+  p.add_argument('--single-commit', action='store_true',
+ help=('run clang-format on a single commit instead of against 
'

lhchavez wrote:
> lodato wrote:
> > nit: I find this flag confusing since it does not follow any git 
> > convention. Instead, I suggest making the interface similar to `git diff`: 
> > if two ``s are given, take the diff of those two trees to find the 
> > list of changed lines, then run clang-format on the second commit.
> > 
> > For example:
> > 
> > * `git clang-format --diff HEAD HEAD~` would tell you if HEAD was properly 
> > clang-formatted or not.
> > * `git clang-format --diff 8bf003 ae9172` would tell you if any of the 
> > lines in ae9172 that differ from 8bf003 are not properly clang-formatted.
> > 
> >  operate in this new mode only if two ``s are given.  Then the 
> > interface would be, for example, `git clang-format abcd1234 abcd1234~`.
> The thing I liked about using git-diff-tree is that you can pass a single 
> commit and even if it's a merge-commit, it does the right thing. I'll try 
> that idea out and allow more than two commits and see how it behaves.
Nevermind, `git diff-tree` didn't work with merge-commits if a single commit is 
passed. Changed to your proposal.


https://reviews.llvm.org/D24319



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

RE: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

[Robinson, Paul via cfe-commits]

The text in the committee draft is different (e.g., the exhortation about 
non-default alignment is gone), with an example to the effect that a value of 8 
means the entity's address is a multiple of 8 (not 2^8).  So, alignment is 
conceived in terms of address bits, whatever those represent (not always bytes).

If Clang is being infested with more target knowledge, okay, but that means 
tolerating the weirder targets in these cases.  Dividing by CHAR_BITS makes an 
assumption that isn't necessarily correct.
--paulr
P.S. The committee is hoping to get a draft out for public comment Real Soon 
Now.

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
David Blaikie via cfe-commits
Sent: Monday, September 12, 2016 5:12 PM
To: reviews+d24426+public+6ee6274d38fdf...@reviews.llvm.org; 
vlesc...@accesssoftek.com; echri...@gmail.com; apra...@apple.com; 
mehdi.am...@apple.com
Cc: cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder 
only if aligment was forced


On Mon, Sep 12, 2016 at 5:00 PM Paul Robinson 
> wrote:
probinson added a subscriber: probinson.


Comment at: lib/CodeGen/CGDebugInfo.cpp:3691
@@ -3635,1 +3690,3 @@
+  if (D->hasAttr())
+AlignInBits = D->getMaxAlignment();
   StringRef DeclName, LinkageName;

dblaikie wrote:
> is max alignment the right thing here? Should it be min alignment?
> (is alignment in bits the desired thing across all of this too? It looked 
> like in the backend patch there was some division by CHAR_BITS, etc?)
I should think bits is the right choice here; seems more the province of the 
backend to convert it into the appropriate addressable units (commonly but not 
universally chars).

The alternative thinking is that we've a generally sense we want to make more 
of this type information opaque to LLVM - so I'm somewhat inclined to make the 
frontend do the work of choosing what to emit and the backend just being as 
simple as possible.

Hmm, seems like the DWARF spec details I can find: 
http://www.dwarfstd.org/ShowIssue.php?issue=140528.1 don't really specify what 
the value of DW_AT_alignment is, it's sort of assumed, by the looks of it? I'm 
assuming it's bytes, the same as the byte_size attribute.





https://reviews.llvm.org/D24426


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

Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.

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

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

Using arcanist to fix the paths as well


https://reviews.llvm.org/D24401

Files:
  tools/clang-format/git-clang-format

Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -77,6 +77,7 @@
   'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
+  'java',  # Java
   'js',  # JavaScript
   'ts',  # TypeScript
   ])


Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -77,6 +77,7 @@
   'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
+  'java',  # Java
   'js',  # JavaScript
   'ts',  # TypeScript
   ])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281285 - Update Clang for D20147 ("DebugInfo: New metadata representation for global variables.")

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Mon Sep 12 20:13:19 2016
New Revision: 281285

URL: http://llvm.org/viewvc/llvm-project?rev=281285=rev
Log:
Update Clang for D20147 ("DebugInfo: New metadata representation for global 
variables.")

Differential Revision: http://reviews.llvm.org/D20415

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/2009-10-20-GlobalDebug.c
cfe/trunk/test/CodeGen/2010-08-10-DbgConstant.c
cfe/trunk/test/CodeGen/debug-info-packed-struct.c
cfe/trunk/test/CodeGen/debug-info-static.c
cfe/trunk/test/CodeGenCXX/debug-info-access.cpp
cfe/trunk/test/CodeGenCXX/debug-info-alias.cpp
cfe/trunk/test/CodeGenCXX/debug-info-anon-namespace.cpp
cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp
cfe/trunk/test/CodeGenCXX/debug-info-cxx1y.cpp
cfe/trunk/test/CodeGenCXX/debug-info-method.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-anonymous-tag.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-ptr-to-member.cpp
cfe/trunk/test/CodeGenCXX/debug-info-ms-vbase.cpp
cfe/trunk/test/CodeGenCXX/debug-info-namespace.cpp
cfe/trunk/test/CodeGenCXX/debug-info-static-member.cpp
cfe/trunk/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp
cfe/trunk/test/CodeGenCXX/debug-info-template.cpp
cfe/trunk/test/CodeGenCXX/debug-info-uuid.cpp
cfe/trunk/test/CodeGenCXX/debug-info.cpp
cfe/trunk/test/CodeGenCXX/debug-lambda-expressions.cpp
cfe/trunk/test/CodeGenCXX/inline-dllexport-member.cpp
cfe/trunk/test/Driver/darwin-debug-flags.c
cfe/trunk/test/Modules/ExtDebugInfo.cpp
cfe/trunk/test/Modules/ExtDebugInfo.m

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281285=281284=281285=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Sep 12 20:13:19 2016
@@ -3606,8 +3606,8 @@ llvm::DIGlobalVariable *CGDebugInfo::Col
 }
 // Use VarDecl's Tag, Scope and Line number.
 GV = DBuilder.createGlobalVariable(DContext, FieldName, LinkageName, Unit,
-   LineNo, FieldTy,
-   Var->hasLocalLinkage(), Var, nullptr);
+   LineNo, FieldTy, 
Var->hasLocalLinkage());
+Var->addDebugInfo(GV);
   }
   return GV;
 }
@@ -3640,14 +3640,14 @@ void CGDebugInfo::EmitGlobalVariable(llv
   } else {
 GV = DBuilder.createGlobalVariable(
 DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, 
Unit),
-Var->hasLocalLinkage(), Var,
+Var->hasLocalLinkage(), /*Expr=*/nullptr,
 getOrCreateStaticDataMemberDeclarationOrNull(D));
+Var->addDebugInfo(GV);
   }
   DeclCache[D->getCanonicalDecl()].reset(GV);
 }
 
-void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,
- llvm::Constant *Init) {
+void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue ) 
{
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
@@ -3687,9 +3687,13 @@ void CGDebugInfo::EmitGlobalVariable(con
   auto  = DeclCache[VD];
   if (GV)
 return;
+  llvm::DIExpression *InitExpr = nullptr;
+  if (Init.isInt())
+InitExpr =
+DBuilder.createConstantValueExpression(Init.getInt().getExtValue());
   GV.reset(DBuilder.createGlobalVariable(
   DContext, Name, StringRef(), Unit, getLineNumber(VD->getLocation()), Ty,
-  true, Init, getOrCreateStaticDataMemberDeclarationOrNull(VarD)));
+  true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD)));
 }
 
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=281285=281284=281285=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon Sep 12 20:13:19 2016
@@ -354,8 +354,8 @@ public:
   /// Emit information about a global variable.
   void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
-  /// Emit global variable's debug info.
-  void EmitGlobalVariable(const ValueDecl *VD, llvm::Constant *Init);
+  /// Emit a constant global variable's debug info.
+  void EmitGlobalVariable(const ValueDecl *VD, const APValue );
 
   /// Emit C++ using directive.
   void EmitUsingDirective(const UsingDirectiveDecl );

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 

[PATCH] D24487: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings in AST; other minor fixes

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko created this revision.
Eugene.Zelenko added reviewers: mehdi_amini, compnerd, Prazek.
Eugene.Zelenko added a subscriber: cfe-commits.
Eugene.Zelenko set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.

I checked this patch on my own build on RHEL 6. Regressions were OK.

Repository:
  rL LLVM

https://reviews.llvm.org/D24487

Files:
  lib/ARCMigrate/Transforms.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/Stmt.cpp
  lib/AST/Type.cpp
  lib/AST/VTableBuilder.cpp
  lib/ASTMatchers/Dynamic/Parser.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp

Index: lib/ASTMatchers/Dynamic/Parser.cpp
===
--- lib/ASTMatchers/Dynamic/Parser.cpp
+++ lib/ASTMatchers/Dynamic/Parser.cpp
@@ -1,4 +1,4 @@
-//===--- Parser.cpp - Matcher expression parser -*- C++ -*-===//
+//===--- Parser.cpp - Matcher expression parser -*- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -13,13 +13,20 @@
 //===--===//
 
 #include "clang/ASTMatchers/Dynamic/Parser.h"
+#include "clang/ASTMatchers/Dynamic/Diagnostics.h"
 #include "clang/ASTMatchers/Dynamic/Registry.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
 
+
 namespace clang {
 namespace ast_matchers {
 namespace dynamic {
@@ -138,7 +145,7 @@
   if (isAlphanumeric(Code[0])) {
 // Parse an identifier
 size_t TokenLength = 1;
-while (1) {
+while (true) {
   // A code completion location in/immediately after an identifier will
   // cause the portion of the identifier before the code completion
   // location to become a code completion token.
@@ -351,7 +358,7 @@
 EndToken = Tokenizer->consumeNextToken();
 break;
   }
-  if (Args.size() > 0) {
+  if (!Args.empty()) {
 // We must find a , token to continue.
 const TokenInfo CommaToken = Tokenizer->consumeNextToken();
 if (CommaToken.Kind != TokenInfo::TK_Comma) {
@@ -607,6 +614,6 @@
   return Result;
 }
 
-}  // namespace dynamic
-}  // namespace ast_matchers
-}  // namespace clang
+} // end namespace dynamic
+} // end namespace ast_matchers
+} // end namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -1,32 +1,39 @@
-//===--- Registry.cpp - Matcher registry -===//
+//===--- Registry.cpp - Matcher registry *- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//======//
+//===--===//
 ///
 /// \file
 /// \brief Registry map populated at static initialization time.
 ///
-//======//
+//===--===//
 
 #include "clang/ASTMatchers/Dynamic/Registry.h"
 #include "Marshallers.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
 
 using namespace clang::ast_type_traits;
 
 namespace clang {
 namespace ast_matchers {
 namespace dynamic {
+
 namespace {
 
 using internal::MatcherDescriptor;
@@ -68,7 +75,7 @@
 }; \
 registerMatcher(#name, \
 new internal::OverloadedMatcherDescriptor(Callbacks)); \
-  } while (0)
+  } while (false)
 
 /// \brief Generate a registry map with all the known matchers.
 RegistryMaps::RegistryMaps() {
@@ -432,7 +439,7 @@
 
 static llvm::ManagedStatic RegistryData;
 
-} // anonymous namespace
+} // end anonymous namespace
 
 // static
 llvm::Optional Registry::lookupMatcherCtor(StringRef MatcherName) {
@@ -461,7 +468,7 @@
   return OS;
 }
 
-}  // namespace
+} // end anonymous namespace
 
 std::vector Registry::getAcceptedCompletionTypes(
 ArrayRef> Context) {
@@ -600,6 +607,6 @@
   return VariantMatcher();
 }
 
-}  // namespace dynamic
-}  // namespace ast_matchers
-}  // namespace clang
+} // end namespace dynamic
+} // end namespace ast_matchers
+} // end namespace 

[PATCH] D24488: Simplify Clang's version number configuration in CMake.

[David L. Jones via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a subscriber: cfe-commits.
Herald added subscribers: mgorny, beanz.

Simplify Clang's version number configuration in CMake.

Currently, the Clang version is computed as follows:

 1. LLVM defines major, minor, and patch versions, all statically set. Today,
these are 4, 0, and 0, respectively.
 2. The static version numbers are combined into PACKAGE_VERSION along with a
suffix, so the result today looks like "4.0.0svn".
 3. Clang extracts CLANG_VERSION from PACKAGE_VERSION using a regexp. The regexp
allows the patch level to omitted, and drops any non-digit trailing values.
Today, this result looks like "4.0.0".
 4. CLANG_VERSION is then split further into CLANG_VERSION_MAJOR and
CLANG_VERSION_MINOR. Today, these resolve to 4 and 0, respectively.
 5. If CLANG_VERSION matches a regexp with three version components, then
CLANG_VERSION_PATCHLEVEL is extracted and the CLANG_HAS_VERSION_PATCHLEVEL
variable is set to 1. Today, these values are 0 and 1, respectively.
 6. The CLANG_VERSION_* variables (and CLANG_HAS_VERSION_PATCHLEVEL) are
configured into [llvm/tools/clang/]include/clang/Basic/Version.inc
verbatim by CMake.
 7. In [llvm/tools/clang/]include/clang/Basic/Version.h, macros are defined
conditionally, based on CLANG_HAS_VERSION_PATCHLEVEL, to compute
CLANG_VERSION_STRING as either a two- or three-level version number. Today,
this value is "4.0.0", because despite the patchlevel being 0, it was
matched by regexp and is thus "HAS"ed by the preprocessor. This string is
then used wherever Clang's "version" is needed [*].

[*] Including, notably, by compiler-rt, for computing its installation path.

This change collapses steps 2-5 by defaulting Clang to use LLVM's (non-string)
version components for the Clang version (see [*] for why not PACKAGE_VERSION),
and collapses steps 6 and 7 by simply writing CLANG_VERSION_STRING into
Version.inc. The Clang version today always uses the patchlevel form, so the
collapsed Version.inc does not have logic for a version without a patch level.

Historically speaking, this technique began with the VER file in r82085 (which
survives in the form of the regexp in #3). The major, minor, and patchlevel
versions were introduced by r106863 (which remains in #4-6). The VER file itself
was deleted in favor of the LLVM version number in r106914. On the LLVM side,
the individual LLVM_VERSION_MAJOR, LLVM_VERSION_MINOR, and PACKAGE_VERSION
weren't introduced for nearly two more years, until r150405.

https://reviews.llvm.org/D24488

Files:
  CMakeLists.txt
  include/clang/Basic/Version.h
  include/clang/Basic/Version.inc.in
  lib/Frontend/InitPreprocessor.cpp

Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -511,16 +511,12 @@
 #define TOSTR(X) TOSTR2(X)
   Builder.defineMacro("__clang_major__", TOSTR(CLANG_VERSION_MAJOR));
   Builder.defineMacro("__clang_minor__", TOSTR(CLANG_VERSION_MINOR));
-#ifdef CLANG_VERSION_PATCHLEVEL
   Builder.defineMacro("__clang_patchlevel__", TOSTR(CLANG_VERSION_PATCHLEVEL));
-#else
-  Builder.defineMacro("__clang_patchlevel__", "0");
-#endif
+#undef TOSTR
+#undef TOSTR2
   Builder.defineMacro("__clang_version__", 
   "\"" CLANG_VERSION_STRING " "
   + getClangFullRepositoryVersion() + "\"");
-#undef TOSTR
-#undef TOSTR2
   if (!LangOpts.MSVCCompat) {
 // Currently claim to be compatible with GCC 4.2.1-5621, but only if we're
 // not compiling for MSVC compatibility
Index: include/clang/Basic/Version.inc.in
===
--- include/clang/Basic/Version.inc.in
+++ include/clang/Basic/Version.inc.in
@@ -1,6 +1,5 @@
 #define CLANG_VERSION @CLANG_VERSION@
+#define CLANG_VERSION_STRING "@CLANG_VERSION@"
 #define CLANG_VERSION_MAJOR @CLANG_VERSION_MAJOR@
 #define CLANG_VERSION_MINOR @CLANG_VERSION_MINOR@
-#if @CLANG_HAS_VERSION_PATCHLEVEL@
 #define CLANG_VERSION_PATCHLEVEL @CLANG_VERSION_PATCHLEVEL@
-#endif
Index: include/clang/Basic/Version.h
===
--- include/clang/Basic/Version.h
+++ include/clang/Basic/Version.h
@@ -19,26 +19,6 @@
 #include "clang/Basic/Version.inc"
 #include "llvm/ADT/StringRef.h"
 
-/// \brief Helper macro for CLANG_VERSION_STRING.
-#define CLANG_MAKE_VERSION_STRING2(X) #X
-
-#ifdef CLANG_VERSION_PATCHLEVEL
-/// \brief Helper macro for CLANG_VERSION_STRING.
-#define CLANG_MAKE_VERSION_STRING(X,Y,Z) CLANG_MAKE_VERSION_STRING2(X.Y.Z)
-
-/// \brief A string that describes the Clang version number, e.g., "1.0".
-#define CLANG_VERSION_STRING \
-  CLANG_MAKE_VERSION_STRING(CLANG_VERSION_MAJOR,CLANG_VERSION_MINOR, \
-CLANG_VERSION_PATCHLEVEL)
-#else
-/// \brief Helper macro for 

Re: [PATCH] D21506: [analyzer] Block in critical section

[Artem Dergachev via cfe-commits]

NoQ added a comment.

I think there's still this problem i've outlined in the comment above: you can 
step into an integer-underflow if your analysis begins with unlock(). You could 
just ignore all unlocks that move you below 0, which would be ok.

Could you add this test?

  std::mutex m;
  void foo() {
m.unlock(); // MutexCount = 4294967295, should be 0, just ignore this 
unlock.
sleep(1); // no-warning
m.lock(); // MutexCount = 0, should be 1, woohoo we're sure we're in the 
section now.
// What's the current mutex count?
sleep(1); // expected-warning{{}}
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D21506



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

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

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

I have made an experiment with a simple kernel:

  void foo1(void);
  void foo2(void);
  void foo3(void);
  void foo4(void);
  void foo5(void);
  void foo6(void);
  void foo7(void);
  void foo8(void);
  void foo9(void);
  void foo10(void);
  
  void test(){
foo1();
foo2();
foo3();
foo4();
foo5();
foo6();
foo7();
foo8();
foo9();
foo10();
  }

I am using time utility of linux to measure the compile time running Clang in 
CL2.0 mode and average over 100 samples. It shows me around 7% overhead with 
your approach.

Even thought it doesn't seem to be large overhead in this test, however it 
would be undesirable to have any regressions in compilation time in the future 
Clang releases for OpenCL basic functionality. Our target should be constantly 
to improve on that unless we have a good reason not to. But in this case I 
don't feel like the reason is strong enough. Also it would be nice to study 
more use cases to see how the compilation time scales. Compilation time might 
be a concern for runtime compilations especially in embedded/constraint 
platform.

Based on this, one alternative I would like to propose - how about we use old 
mechanism for standard OpenCL extensions that come from the Spec and dynamic 
maps for vendor extensions? We can then have an early return to avoid the 
expensive checks if  the maps of extensions are empty (OpenCLTypeExtMap, 
OpenCLDeclExtMap) or full check if they contain any elements. In this case if 
vendors are fine with having slight overhead of compilation time they can use 
the feature to add their extensions flexibly, otherwise they will have to stay 
with an old approach of extending Clang directly. What do you think about it? 
Any other proposals are welcome too!


https://reviews.llvm.org/D21698



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

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

[David Blaikie via cfe-commits]

On Mon, Sep 12, 2016 at 6:01 PM Robinson, Paul 
wrote:

> The text in the committee draft is different (e.g., the exhortation about
> non-default alignment is gone), with an example to the effect that a value
> of 8 means the entity's address is a multiple of 8 (not 2^8).  So,
> alignment is conceived in terms of address bits, whatever those represent
> (not always bytes).
>
Not sure I quite follow. OK, so in an octet addressable world (which LLVM
is - there have been some attempts to support non-octet addressing, but I
don't think any have been near to successful) then DW_AT_alignment is byte
alignment (1 means there are no zero bits in the address, 2 means there's 1
trailing zero bit in the address, etc).

> If Clang is being infested with more target knowledge, okay, but that
> means tolerating the weirder targets in these cases.  Dividing by CHAR_BITS
> makes an assumption that isn't necessarily correct.
>
Clang has the knowledge already - it knows the alignment of the types its
allocating, etc. So I'm not sure what infestation you're referring to.

I've sort of lost track of what we're discussing here.

Essentially what I'm suggesting is that Clang should put whatever number is
going to go in the DWARF, into the metadata. I don't believe the LLVM
backends have greater knowledge than the frontend does in this domain -
have I missed something there, are there examples where that could/would be
true?

- David


> --paulr
>
> P.S. The committee is hoping to get a draft out for public comment Real
> Soon Now.
>
Looking forward to it :)

>
>
> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
> Behalf Of *David Blaikie via cfe-commits
> *Sent:* Monday, September 12, 2016 5:12 PM
> *To:* reviews+d24426+public+6ee6274d38fdf...@reviews.llvm.org;
> vlesc...@accesssoftek.com; echri...@gmail.com; apra...@apple.com;
> mehdi.am...@apple.com
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to
> DIBuilder only if aligment was forced
>
>
>
>
>
> On Mon, Sep 12, 2016 at 5:00 PM Paul Robinson 
> wrote:
>
> probinson added a subscriber: probinson.
>
> 
> Comment at: lib/CodeGen/CGDebugInfo.cpp:3691
> @@ -3635,1 +3690,3 @@
> +  if (D->hasAttr())
> +AlignInBits = D->getMaxAlignment();
>StringRef DeclName, LinkageName;
> 
> dblaikie wrote:
> > is max alignment the right thing here? Should it be min alignment?
> > (is alignment in bits the desired thing across all of this too? It
> looked like in the backend patch there was some division by CHAR_BITS, etc?)
> I should think bits is the right choice here; seems more the province of
> the backend to convert it into the appropriate addressable units (commonly
> but not universally chars).
>
>
> The alternative thinking is that we've a generally sense we want to make
> more of this type information opaque to LLVM - so I'm somewhat inclined to
> make the frontend do the work of choosing what to emit and the backend just
> being as simple as possible.
>
> Hmm, seems like the DWARF spec details I can find:
> http://www.dwarfstd.org/ShowIssue.php?issue=140528.1 don't really specify
> what the value of DW_AT_alignment is, it's sort of assumed, by the looks of
> it? I'm assuming it's bytes, the same as the byte_size attribute.
>
>
>
>
>
>
> https://reviews.llvm.org/D24426
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281286 - Fix interaction between serialization and c++1z feature.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Mon Sep 12 20:20:40 2016
New Revision: 281286

URL: http://llvm.org/viewvc/llvm-project?rev=281286=rev
Log:
Fix interaction between serialization and c++1z feature.

In c++1z, static_assert is not required to have a StringLiteral message, where
previously it was required.  Update the AST Reader to be able to handle a
null StringLiteral.

Added:
cfe/trunk/test/Modules/Inputs/static_assert/
cfe/trunk/test/Modules/Inputs/static_assert/a.h
cfe/trunk/test/Modules/Inputs/static_assert/module.modulemap
cfe/trunk/test/Modules/static_assert.cpp
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=281286=281285=281286=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Sep 12 20:20:40 2016
@@ -2239,7 +2239,7 @@ void ASTDeclReader::VisitStaticAssertDec
   VisitDecl(D);
   D->AssertExprAndFailed.setPointer(Reader.ReadExpr(F));
   D->AssertExprAndFailed.setInt(Record[Idx++]);
-  D->Message = cast(Reader.ReadExpr(F));
+  D->Message = cast_or_null(Reader.ReadExpr(F));
   D->RParenLoc = ReadSourceLocation(Record, Idx);
 }
 

Added: cfe/trunk/test/Modules/Inputs/static_assert/a.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/static_assert/a.h?rev=281286=auto
==
--- cfe/trunk/test/Modules/Inputs/static_assert/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/static_assert/a.h Mon Sep 12 20:20:40 2016
@@ -0,0 +1,3 @@
+class S {
+  static_assert(4 == 4);
+};

Added: cfe/trunk/test/Modules/Inputs/static_assert/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/static_assert/module.modulemap?rev=281286=auto
==
--- cfe/trunk/test/Modules/Inputs/static_assert/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/static_assert/module.modulemap Mon Sep 12 
20:20:40 2016
@@ -0,0 +1 @@
+module a { header "a.h" }

Added: cfe/trunk/test/Modules/static_assert.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/static_assert.cpp?rev=281286=auto
==
--- cfe/trunk/test/Modules/static_assert.cpp (added)
+++ cfe/trunk/test/Modules/static_assert.cpp Mon Sep 12 20:20:40 2016
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps \
+// RUN:-I%S/Inputs/static_assert -std=c++1z -verify %s
+// expected-no-diagnostics
+
+#include "a.h"
+
+S s;


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

Re: [PATCH] D15075: No error for conflict between inputs\outputs and clobber list

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

You can create a separate patch for the changes made to lib/Headers/intrin.h 
and have it reviewed before committing this patch.

Also, __dst, __x and __n should be added to the output list and "memory" to the 
clobber list as majnemer pointed out. I think you can use constraint "+" to 
ensure the registers are clobbered. For example:

  __asm__("rep movsb" : "+D"(__dst), "+S"(__src), "+c"(__n) : : "memory");

For stos, "a"(__x) can remain an input constraint since it doesn't get modified.


https://reviews.llvm.org/D15075



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

r281287 - Handle empty message in static_asserts.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Mon Sep 12 20:37:01 2016
New Revision: 281287

URL: http://llvm.org/viewvc/llvm-project?rev=281287=rev
Log:
Handle empty message in static_asserts.

Modified:
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=281287=281286=281287=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Mon Sep 12 20:37:01 2016
@@ -1243,8 +1243,9 @@ bool CursorVisitor::VisitUnresolvedUsing
 bool CursorVisitor::VisitStaticAssertDecl(StaticAssertDecl *D) {
   if (Visit(MakeCXCursor(D->getAssertExpr(), StmtParent, TU, 
RegionOfInterest)))
 return true;
-  if (Visit(MakeCXCursor(D->getMessage(), StmtParent, TU, RegionOfInterest)))
-return true;
+  if (StringLiteral *Message = D->getMessage())
+if (Visit(MakeCXCursor(Message, StmtParent, TU, RegionOfInterest)))
+  return true;
   return false;
 }
 


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

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Meike Baumgärtner via cfe-commits]

meikeb marked 6 inline comments as done.
meikeb added a comment.

Thanks for taking the time and doing these great reviews! Really appreciated!



Comment at: lib/Sema/SemaChecking.cpp:4143-4150
@@ -4049,3 +4142,10 @@
 if (StrE) {
-  CheckFormatString(S, StrE, E, Args, HasVAListArg, format_idx,
+  if (Offset.isNegative() || Offset > StrE->getLength()) {
+// TODO: It would be better to have an explicit warning for out of
+// bounds literals.
+return SLCT_NotALiteral;
+  }
+  FormatStringLiteral FStr =
+  FormatStringLiteral(StrE, Offset.sextOrTrunc(64).getSExtValue());
+  CheckFormatString(S, , E, Args, HasVAListArg, format_idx,
 firstDataArg, Type, InFunctionCall, CallType,

The = case is part of a different warning. It's checked in CheckFormatString.


https://reviews.llvm.org/D23820



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

Re: [PATCH] D24439: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings; other minor fixes

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko updated this revision to Diff 71063.
Eugene.Zelenko added a comment.

Implement Saleem suggestions.


Repository:
  rL LLVM

https://reviews.llvm.org/D24439

Files:
  lib/Analysis/PrintfFormatString.cpp
  lib/Basic/Diagnostic.cpp
  lib/Basic/SourceManager.cpp
  lib/Index/IndexDecl.cpp
  lib/Index/IndexTypeSourceInfo.cpp
  lib/Lex/PTHLexer.cpp
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Rewrite/DeltaTree.cpp
  lib/Rewrite/HTMLRewrite.cpp

Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -13,10 +13,27 @@
 
 #include "clang/Parse/Parser.h"
 #include "RAIIObjectsForParser.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/Token.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+#include 
+#include 
+#include 
+
 using namespace clang;
 
 /// ParseCXXInlineMethodDef - We parsed and verified that the specified
@@ -672,7 +689,7 @@
   // We always want this function to consume at least one token if the first
   // token isn't T and if not at EOF.
   bool isFirstTokenConsumed = true;
-  while (1) {
+  while (true) {
 // If we found one of the tokens, stop and return true.
 if (Tok.is(T1) || Tok.is(T2)) {
   if (ConsumeFinalToken) {
@@ -1012,7 +1029,7 @@
   unsigned AngleCount = 0;
   unsigned KnownTemplateCount = 0;
 
-  while (1) {
+  while (true) {
 switch (Tok.getKind()) {
 case tok::comma:
   // If we might be in a template, perform a tentative parse to check.
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -23,13 +23,39 @@
 
 #include "RAIIObjectsForParser.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/OperatorPrecedence.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Basic/TypeTraits.h"
+#include "clang/Basic/VersionTuple.h"
+#include "clang/Lex/Token.h"
 #include "clang/Parse/Parser.h"
+#include "clang/Sema/AttributeList.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/TypoCorrection.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
+#include 
+#include 
+#include 
+
 using namespace clang;
 
 /// \brief Simple precedence-based parser for binary/ternary operators.
@@ -251,7 +277,7 @@
getLangOpts().CPlusPlus11);
   SourceLocation ColonLoc;
 
-  while (1) {
+  while (true) {
 // If this token has a lower precedence than we are allowed to parse (e.g.
 // because we are called recursively, or because the token is not a binop),
 // then we are done!
@@ -485,8 +511,9 @@
 }
 
 namespace {
+
 class CastExpressionIdValidator : public CorrectionCandidateCallback {
- public:
+public:
   CastExpressionIdValidator(Token Next, bool AllowTypes, bool AllowNonTypes)
   : NextToken(Next), AllowNonTypes(AllowNonTypes) {
 WantTypeSpecifiers = WantFunctionLikeCasts = AllowTypes;
@@ -514,11 +541,12 @@
 return false;
   }
 
- private:
+private:
   Token NextToken;
   bool AllowNonTypes;
 };
-}
+
+} // end anonymous namespace
 
 /// \brief Parse a cast-expression, or, if \pisUnaryExpression is true, parse
 /// a unary-expression.
@@ -1407,7 +1435,7 @@
   // Now that the primary-expression piece of the postfix-expression has been
   // parsed, see if there are any postfix-expression pieces here.
   SourceLocation Loc;
-  while (1) {
+  while (true) {
 switch (Tok.getKind()) {
 case tok::code_completion:
   if (InMessageExpression)
@@ -1810,7 +1838,6 @@
   return Operand;
 }
 
-
 /// \brief Parse a sizeof or alignof expression.
 ///
 /// \verbatim
@@ -2004,7 +2031,7 @@
 Comps.back().LocStart = Comps.back().LocEnd = ConsumeToken();
 
 // FIXME: This 

Re: [PATCH] D24439: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings; other minor fixes

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko marked 3 inline comments as done.
Eugene.Zelenko added a comment.

Repository:
  rL LLVM

https://reviews.llvm.org/D24439



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

Re: [PATCH] D23852: [SemaObjC] Fix crash while parsing type arguments and protocols

[Doug Gregor via cfe-commits]

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

I don't love the fact that it makes callers fragile, but having to do tentative 
parsing for these otherwise-unambiguous cases is expensive (in compile time). 
We should only use tentative parsing when we have an actual ambiguity to 
resolve.


https://reviews.llvm.org/D23852



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

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

[David Blaikie via cfe-commits]

dblaikie added a comment.

Like the backend patch, should/could this be broken up into separate patches 
for the different places/features that have alignment?

(probably just passing zero for alignment in general could be a simple cleanup 
patch to start, if it's otherwise unused)



Comment at: lib/CodeGen/CGDebugInfo.cpp:609
@@ -609,3 +608,3 @@
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
-  uint64_t Align = CGM.getContext().getTypeAlign(Ty);
+  uint64_t Align = 0;
   return DBuilder.createBasicType("complex", Size, Align, Encoding);

Maybe add a comment here about why we're specifying zero alignment (or possibly 
change the API to not take an alignment at all so we don't have to explain it?)

(similarly for other cases where the alignment is just hardcoded to zero)


Comment at: lib/CodeGen/CGDebugInfo.cpp:3691
@@ -3635,1 +3690,3 @@
+  if (D->hasAttr())
+AlignInBits = D->getMaxAlignment();
   StringRef DeclName, LinkageName;

is max alignment the right thing here? Should it be min alignment?
(is alignment in bits the desired thing across all of this too? It looked like 
in the backend patch there was some division by CHAR_BITS, etc?)


https://reviews.llvm.org/D24426



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

Re: [PATCH] D23079: ObjC: Use a new type for ObjC type parameter (patch 2 out of 3)

[Doug Gregor via cfe-commits]

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

This looks great, thank you!


https://reviews.llvm.org/D23079



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

Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit

[Mark Lodato via cfe-commits]

lodato added a subscriber: lodato.
lodato added a comment.

Hi lhchavez,

This patch does not work as intended. If I understand correctly, you want to 
see if a given `` has any clang-format problems. However:

1. This patch only computes `changed_lines` from `` but then runs 
clang-format on the files in the working directory. Instead, you need to 
somehow update `run_clang_format_and_save_to_tree()` to operate on ``.

2. Unless `--diff` is given, this new mode writes clang-formatted contents of 
`` to the working directory, blowing away any changes since then. The 
simple solution is to require `--diff` whenever this new mode is used, since 
it's not obvious what should be written to the working directory.

See also D15465 , which had similar problems.

Here is an example showing the problem. The last command should have said that 
aa207 was bad, but since the current working directory is fine, it shows no 
diff.

  $ git log -p --reverse
  commit a2765ba80f03f02aaa0cefbfe705ae844c219065
  Author: ...
  Date:   2016-09-12 16:29:53 -0400
  
  initial commit; x and y bad
  
  diff --git a/foo.cc b/foo.cc
  new file mode 100644
  index 000..08422a0
  --- /dev/null
  +++ b/foo.cc
  @@ -0,0 +1,4 @@
  +void foo() {
  +  int x=1;
  +  int y=2;
  +}
  
  commit aa207f7991d26d1ee6826bf272f8eefffdf31b31
  Author: ...
  Date:   2016-09-12 16:30:19 -0400
  
  modify x, still bad
  
  diff --git a/foo.cc b/foo.cc
  index 08422a0..dd75f40 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,4 +1,4 @@
   void foo() {
  -  int x=1;
  +  int x=3;
 int y=2;
   }
  
  commit 72858d5d2b13ed9dcf6a3328fef43a81ee3f01fc
  Author: ...
  Date:   2016-09-12 16:30:58 -0400
  
  modify both lines, still bad
  
  diff --git a/foo.cc b/foo.cc
  index dd75f40..3eff9ca 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,4 +1,4 @@
   void foo() {
  -  int x=3;
  -  int y=2;
  +  int x=30;
  +  int y=20;
   }
  
  commit 9e6bfa1b9582b1423efc6d2339e269636fa0718b
  Author: ...
  Date:   2016-09-12 16:32:21 -0400
  
  clang-format
  
  diff --git a/foo.cc b/foo.cc
  index 3eff9ca..ac4d00d 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,4 +1,4 @@
   void foo() {
  -  int x=30;
  -  int y=20;
  +  int x = 30;
  +  int y = 20;
   }
  $ ~/tmp/git-clang-format --single-commit aa207
  clang-format did not modify any files



Comment at: cfe/trunk/tools/clang-format/git-clang-format:93
@@ -92,1 +92,3 @@
  help='default commit to use if none is specified'),
+  p.add_argument('--single-commit', action='store_true',
+ help=('run clang-format on a single commit instead of against 
'

nit: I find this flag confusing since it does not follow any git convention. 
Instead, I suggest making the interface similar to `git diff`: if two 
``s are given, take the diff of those two trees to find the list of 
changed lines, then run clang-format on the second commit.

For example:

* `git clang-format --diff HEAD HEAD~` would tell you if HEAD was properly 
clang-formatted or not.
* `git clang-format --diff 8bf003 ae9172` would tell you if any of the lines in 
ae9172 that differ from 8bf003 are not properly clang-formatted.

 operate in this new mode only if two ``s are given.  Then the 
interface would be, for example, `git clang-format abcd1234 abcd1234~`.


Comment at: cfe/trunk/tools/clang-format/git-clang-format:323
@@ -312,1 +322,3 @@
 
+def create_tree_from_commit(commit, filenames):
+  """Create a new git tree with the given files from `commit`.

Unless I'm mistaken, this function is unnecessary. There is no need to filter 
out files in the tree that do not match the pattern, since the filtering 
happens in `compute_diff()` (`cmd.extend(files)`).


Repository:
  rL LLVM

https://reviews.llvm.org/D24319



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

Re: [PATCH] D23080: ObjC: Use a new type for ObjC type parameter (patch 3 out of 3)

[Doug Gregor via cfe-commits]

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

Ahhh, much cleaner. Thanks!


https://reviews.llvm.org/D23080



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

Re: [PATCH] D24059: NFC: refactor applyObjCProtocolQualifiers from SemaType.cpp to ASTContext so it can be shared.

[Doug Gregor via cfe-commits]

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

Yep, this refactor looks good!


https://reviews.llvm.org/D24059



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

r281275 - Fix a long comment line

[Adam Nemet via cfe-commits]

Author: anemet
Date: Mon Sep 12 18:48:11 2016
New Revision: 281275

URL: http://llvm.org/viewvc/llvm-project?rev=281275=rev
Log:
Fix a long comment line

Modified:
cfe/trunk/include/clang/Frontend/CodeGenOptions.def

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=281275=281274=281275=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Mon Sep 12 18:48:11 2016
@@ -1,4 +1,4 @@
-//===--- CodeGenOptions.def - Code generation option database -- C++ 
-*-===//
+//===--- CodeGenOptions.def - Code generation option database - C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //


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

r281276 - Add -fdiagnostics-show-hotness

[Adam Nemet via cfe-commits]

Author: anemet
Date: Mon Sep 12 18:48:16 2016
New Revision: 281276

URL: http://llvm.org/viewvc/llvm-project?rev=281276=rev
Log:
Add -fdiagnostics-show-hotness

Summary:
I've recently added the ability for optimization remarks to include the
hotness of the corresponding code region.  This uses PGO and allows
filtering of the optimization remarks by relevance.  The idea was first
discussed here:
http://thread.gmane.org/gmane.comp.compilers.llvm.devel/98334

The general goal is to produce a YAML file with the remarks.  Then, an
external tool could dynamically filter these by hotness and perhaps by
other things.

That said it makes sense to also expose this at the more basic level
where we just include the hotness info with each optimization remark.
For example, in D22694, the clang flag was pretty useful to measure the
overhead of the additional analyses required to include hotness.
(Without the flag we don't even run the analyses.)

For the record, Hal has already expressed support for the idea of this
patch on IRC.

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

Added:
cfe/trunk/test/Frontend/Inputs/optimization-remark-with-hotness.proftext
cfe/trunk/test/Frontend/optimization-remark-with-hotness.c
Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=281276=281275=281276=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Sep 12 18:48:16 2016
@@ -317,6 +317,28 @@ output format of the diagnostics that it
by category, so it should be a high level category. We want dozens
of these, not hundreds or thousands of them.
 
+.. _opt_fdiagnostics-show-hotness:
+
+**-f[no-]diagnostics-show-hotness**
+   Enable profile hotness information in diagnostic line.
+
+   This option, which defaults to off, controls whether Clang prints the
+   profile hotness associated with a diagnostics in the presence of
+   profile-guided optimization information.  This is currently supported with
+   optimization remarks (see :ref:`Options to Emit Optimization Reports
+   `).  The hotness information allows users to focus on the hot
+   optimization remarks that are likely to be more relevant for run-time
+   performance.
+
+   For example, in this output, the block containing the callsite of `foo` was
+   executed 3000 times according to the profile data:
+
+   ::
+
+ s.c:7:10: remark: foo inlined into bar (hotness: 3000) 
[-Rpass-analysis=inline]
+   sum += foo(x, x - 2);
+  ^
+
 .. _opt_fdiagnostics-fixit-info:
 
 **-f[no-]diagnostics-fixit-info**
@@ -535,6 +557,8 @@ control the crash diagnostics.
 The -fno-crash-diagnostics flag can be helpful for speeding the process
 of generating a delta reduced test case.
 
+.. _rpass:
+
 Options to Emit Optimization Reports
 
 
@@ -578,6 +602,10 @@ outside of the major transformations (e.
 loop optimizations) and not every optimization pass supports this
 feature.
 
+Note that when using profile-guided optimization information, profile hotness
+information can be included in the remarks (see
+:ref:`-fdiagnostics-show-hotness `).
+
 Current limitations
 ^^^
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=281276=281275=281276=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Sep 12 18:48:16 
2016
@@ -189,6 +189,9 @@ def warn_drv_unused_argument : Warning<
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup;
+def warn_drv_fdiagnostics_show_hotness_requires_pgo : Warning<
+  "argument '-fdiagnostics-show-hotness' requires profile-guided optimization 
information">,
+  InGroup;
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support '%0'">;
 def warn_drv_deprecated_arg : Warning<

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=281276=281275=281276=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Sep 12 18:48:16 2016
@@ -583,6 +583,8 @@ def fdiagnostics_parseable_fixits : Flag
 def 

Re: [PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2258
@@ -2257,1 +2257,3 @@
   "uuid attribute contains a malformed GUID">;
+def err_mismatched_uuid : Error<"uiid does not match previous declaration">;
+def note_previous_uuid : Note<"previous uuid specified here">;

uuid instead of uiid.


Comment at: lib/Parse/ParseDecl.cpp:1476
@@ +1475,3 @@
+  // Find end of type attributes Attrs and add NewTypeAttributes in the same
+  // order they were in originally.  (Remember, in AttributeList things earlier
+  // in source order are later in the list, since new attributes are added to

Remove double space here.


Comment at: lib/Sema/SemaDecl.cpp:2255
@@ -2249,1 +2254,3 @@
+  // previous decl", for example if the attribute needs to be consistent
+  // between redeclarations, you need to call a custom merge function here.
   InheritableAttr *NewAttr = nullptr;

Thank you for the improved comments as a drive-by.


Comment at: lib/Sema/SemaDeclAttr.cpp:4609
@@ +4608,3 @@
+  unsigned AttrSpellingListIndex, StringRef Uuid) {
+  if (UuidAttr *UA = D->getAttr()) {
+if (UA->getGuid() == Uuid)

Can use `const auto *` here.

Also, don't you need to iterate over all of the `UuidAttr` objects attached to 
the declaration to see if any of them match, rather than just the first?


https://reviews.llvm.org/D24469



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

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Zhao, Weiming via cfe-commits]

Sorry, I was distracted by other issues after I uploaded the patch.

I will take another look of the implementation.

Thanks,

Weiming


On 9/12/2016 1:31 PM, Sam Shepperd wrote:

phabricss added a comment.

On 09/12/2016 01:26 PM, Nick Lewycky wrote:


Firstly, I thought glibc had applied a patch to fix this bug? As in, the error 
is correct and glibc fixed their bug?


I can confirm that the bug still exists in glibc 2.24 and HEAD from glibc git.  
Also it appears that the issue on the llvm mailing list was just dropped 
without any resolution:

r255371 - Error on redeclaring with a conflicting asm label 


   ../include/sys/stat.h:16:15: error: cannot apply asm label to function after 
its first use
   hidden_proto (__fxstat)
   ~~^
   ./../include/libc-symbols.h:420:19: note: expanded from macro 'hidden_proto'
 __hidden_proto (name, , __GI_##name, ##attrs)
 ^
   ./../include/libc-symbols.h:424:33: note: expanded from macro 
'__hidden_proto'
 extern thread __typeof (name) name __asm__ (__hidden_asmname (#internal)) \

As far as your review of the patch by weimingz, that is beyond my skill level 
so I will let him reply.  Thanks!


https://reviews.llvm.org/D16171





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux Foundation

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

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

[Paul Robinson via cfe-commits]

probinson added a subscriber: probinson.


Comment at: lib/CodeGen/CGDebugInfo.cpp:3691
@@ -3635,1 +3690,3 @@
+  if (D->hasAttr())
+AlignInBits = D->getMaxAlignment();
   StringRef DeclName, LinkageName;

dblaikie wrote:
> is max alignment the right thing here? Should it be min alignment?
> (is alignment in bits the desired thing across all of this too? It looked 
> like in the backend patch there was some division by CHAR_BITS, etc?)
I should think bits is the right choice here; seems more the province of the 
backend to convert it into the appropriate addressable units (commonly but not 
universally chars).


https://reviews.llvm.org/D24426



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

Re: r281057 - [DebugInfo] Ensure complete type is emitted with -fstandalone-debug

[Reid Kleckner via cfe-commits]

On Mon, Sep 12, 2016 at 11:41 AM, David Blaikie  wrote:

> (idle thought: It'd be good to make the code common between the first time
> a type is encountered, and the later callbacks when the type is completed,
> required to be complete, vtable is emitted, etc - the inconsistency here (&
> it's spread out through several functions: complete[Required]Type,
> shouldOmitDefinition, isDefinedInClangModule, etc) is a bit
> problematic/error prone)
>

I went for that in r281278.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.

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

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

Generated the diff properly this time, and it now shows full context.


Repository:
  rL LLVM

https://reviews.llvm.org/D24401

Files:
  cfe/trunk/tools/clang-format/git-clang-format

Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -77,6 +77,7 @@
   'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
+  'java',  # Java
   'js',  # JavaScript
   'ts',  # TypeScript
   ])


Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -77,6 +77,7 @@
   'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp',  # C++
   # Other languages that clang-format supports
   'proto', 'protodevel',  # Protocol Buffers
+  'java',  # Java
   'js',  # JavaScript
   'ts',  # TypeScript
   ])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24484: [analyzer] Fix ExprEngine::VisitMemberExpr

[Alexander Shaposhnikov via cfe-commits]

alexshap created this revision.
alexshap added reviewers: NoQ, bcraig, zaks.anna.
alexshap added a subscriber: cfe-commits.
alexshap set the repository for this revision to rL LLVM.
alexshap changed the visibility of this Differential Revision from "Public (No 
Login Required)" to "All Users".

AST may contain intermediate ParenExpr nodes between MemberExpr and 
ArrayToPointerDecay.
This diff adjusts the check in ExprEngine::VisitMemberExpr accordingly. 
Test plan: make -j8 check-clang-analysis 

Repository:
  rL LLVM

https://reviews.llvm.org/D24484

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/array-struct.c

Index: test/Analysis/array-struct.c
===
--- test/Analysis/array-struct.c
+++ test/Analysis/array-struct.c
@@ -135,6 +135,17 @@
 
 void bar(int*);
 
+struct s3 gets3() {
+  struct s3 s;
+  return s;
+}
+
+void accessArrayFieldNoCrash() {
+  bar(gets3().a);
+  bar((gets3().a));
+  bar(((gets3().a)));  
+}
+
 // Test if the array is correctly invalidated.
 void f15() {
   int a[10];
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2044,7 +2044,7 @@
 if (!M->isGLValue()) {
   assert(M->getType()->isArrayType());
   const ImplicitCastExpr *PE =
-dyn_cast((*I)->getParentMap().getParent(M));
+
dyn_cast((*I)->getParentMap().getParentIgnoreParens(M));
   if (!PE || PE->getCastKind() != CK_ArrayToPointerDecay) {
 llvm_unreachable("should always be wrapped in 
ArrayToPointerDecay");
   }


Index: test/Analysis/array-struct.c
===
--- test/Analysis/array-struct.c
+++ test/Analysis/array-struct.c
@@ -135,6 +135,17 @@
 
 void bar(int*);
 
+struct s3 gets3() {
+  struct s3 s;
+  return s;
+}
+
+void accessArrayFieldNoCrash() {
+  bar(gets3().a);
+  bar((gets3().a));
+  bar(((gets3().a)));  
+}
+
 // Test if the array is correctly invalidated.
 void f15() {
   int a[10];
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2044,7 +2044,7 @@
 if (!M->isGLValue()) {
   assert(M->getType()->isArrayType());
   const ImplicitCastExpr *PE =
-dyn_cast((*I)->getParentMap().getParent(M));
+dyn_cast((*I)->getParentMap().getParentIgnoreParens(M));
   if (!PE || PE->getCastKind() != CK_ArrayToPointerDecay) {
 llvm_unreachable("should always be wrapped in ArrayToPointerDecay");
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24484: [analyzer] Fix ExprEngine::VisitMemberExpr

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

1.

For the code:

  struct TestBufferStruct {
int Buffer[3];
  };
  
  struct TestBufferStruct GetTestBufferStruct() {
struct TestBufferStruct a;
return a;
  }
  
  void AcceptPointer(int *a) {
  }
  
  void TestFunc() {
 AcceptArray(((GetTestBufferStruct().Buffer)));
  }

AST looks like this:

  `-ImplicitCastExpr 0x103879640  'int *' 
 `-ParenExpr 0x1038795b0  'int [3]'
   `-ParenExpr 0x103879590  'int [3]'
 `-MemberExpr 0x103879558  'int [3]' .Buffer 0x103833598
   `-CallExpr 0x103879530  'struct 
TestBufferStruct':'struct TestBufferStruct'
 `-ImplicitCastExpr 0x103879518  'struct TestBufferStruct 
(*)()' 
   `-DeclRefExpr 0x103879498  'struct TestBufferStruct ()' 
Function 0x103878eb8 'GetTestBufferStruct' 'struct 

1.

Test plan: make -j8 check-clang-analysis.
Without my changes it crashes (on the modified test):
F2423801: Screen Shot 2016-09-12 at 5.09.47 PM.png 

With the patch all the tests are green.


Repository:
  rL LLVM

https://reviews.llvm.org/D24484



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

[PATCH] D24477: clang-format: [JS] Do not wrap taze: IWYU comments

[Martin Probst via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: djasper.
mprobst added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

See (sorry, internal link): go/taze#global-symbols-dts-externs.

Before:
// taze: many, different, symbols from
// //some/long/iwyu/style/target

After:
// taze: many, different, symbols from //some/long/iwyu/style/target

https://reviews.llvm.org/D24477

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1429,5 +1429,10 @@
"}");
 }
 
+TEST_F(FormatTestJS, ImportComments) {
+  verifyFormat("import {x} from 'x';  // from //x:x", 
getGoogleJSStyleWithColumns(25));
+  verifyFormat("xx;  // taze: x from //x:x", getGoogleJSStyleWithColumns(10));
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -613,7 +613,7 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-GoogleStyle.CommentPragmas = "@(export|requirecss|return|see|visibility) ";
+GoogleStyle.CommentPragmas = 
"(taze:|@(export|requirecss|return|see|visibility)) ";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1429,5 +1429,10 @@
"}");
 }
 
+TEST_F(FormatTestJS, ImportComments) {
+  verifyFormat("import {x} from 'x';  // from //x:x", getGoogleJSStyleWithColumns(25));
+  verifyFormat("xx;  // taze: x from //x:x", getGoogleJSStyleWithColumns(10));
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -613,7 +613,7 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-GoogleStyle.CommentPragmas = "@(export|requirecss|return|see|visibility) ";
+GoogleStyle.CommentPragmas = "(taze:|@(export|requirecss|return|see|visibility)) ";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23284: Add -fdiagnostics-show-hotness

[Adam Nemet via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281276: Add -fdiagnostics-show-hotness (authored by anemet).

Changed prior to commit:
  https://reviews.llvm.org/D23284?vs=70547=71078#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23284

Files:
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Frontend/Inputs/optimization-remark-with-hotness.proftext
  cfe/trunk/test/Frontend/optimization-remark-with-hotness.c

Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -179,6 +179,7 @@
   Ctx.getDiagnosticHandler();
   void *OldDiagnosticContext = Ctx.getDiagnosticContext();
   Ctx.setDiagnosticHandler(DiagnosticHandler, this);
+  Ctx.setDiagnosticHotnessRequested(CodeGenOpts.DiagnosticsWithHotness);
 
   // Link LinkModule into this module if present, preserving its validity.
   for (auto  : LinkModules) {
@@ -511,9 +512,16 @@
   FullSourceLoc Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename,
   Line, Column);
 
+  std::string Msg;
+  raw_string_ostream MsgStream(Msg);
+  MsgStream << D.getMsg().str();
+
+  if (D.getHotness())
+MsgStream << " (hotness: " << *D.getHotness() << ")";
+
   Diags.Report(Loc, DiagID)
   << AddFlagValue(D.getPassName() ? D.getPassName() : "")
-  << D.getMsg().str();
+  << MsgStream.str();
 
   if (BadDebugInfo)
 // If we were not able to translate the file:line:col information
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -4902,6 +4902,7 @@
   claimNoWarnArgs(Args);
 
   Args.AddAllArgs(CmdArgs, options::OPT_R_Group);
+
   Args.AddAllArgs(CmdArgs, options::OPT_W_Group);
   if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false))
 CmdArgs.push_back("-pedantic");
@@ -5904,6 +5905,10 @@
 CmdArgs.push_back(A->getValue());
   }
 
+  if (Args.hasFlag(options::OPT_fdiagnostics_show_hotness,
+   options::OPT_fno_diagnostics_show_hotness, false))
+CmdArgs.push_back("-fdiagnostics-show-hotness");
+
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -839,6 +839,12 @@
 NeedLocTracking = true;
   }
 
+  Opts.DiagnosticsWithHotness =
+  Args.hasArg(options::OPT_fdiagnostics_show_hotness);
+  if (Opts.DiagnosticsWithHotness &&
+  Opts.getProfileUse() == CodeGenOptions::ProfileNone)
+Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
+
   // If the user requested to use a sample profile for PGO, then the
   // backend will need to track source location information so the profile
   // can be incorporated into the IR.
Index: cfe/trunk/docs/UsersManual.rst
===
--- cfe/trunk/docs/UsersManual.rst
+++ cfe/trunk/docs/UsersManual.rst
@@ -317,6 +317,28 @@
by category, so it should be a high level category. We want dozens
of these, not hundreds or thousands of them.
 
+.. _opt_fdiagnostics-show-hotness:
+
+**-f[no-]diagnostics-show-hotness**
+   Enable profile hotness information in diagnostic line.
+
+   This option, which defaults to off, controls whether Clang prints the
+   profile hotness associated with a diagnostics in the presence of
+   profile-guided optimization information.  This is currently supported with
+   optimization remarks (see :ref:`Options to Emit Optimization Reports
+   `).  The hotness information allows users to focus on the hot
+   optimization remarks that are likely to be more relevant for run-time
+   performance.
+
+   For example, in this output, the block containing the callsite of `foo` was
+   executed 3000 times according to the profile data:
+
+   ::
+
+ s.c:7:10: remark: foo inlined into bar (hotness: 3000) [-Rpass-analysis=inline]
+   sum += foo(x, x - 2);
+  ^
+
 .. _opt_fdiagnostics-fixit-info:
 
 **-f[no-]diagnostics-fixit-info**
@@ -535,6 +557,8 @@
 The -fno-crash-diagnostics flag can be helpful for speeding the process
 of generating a delta reduced test case.
 
+.. _rpass:
+
 Options to Emit Optimization Reports
 
 
@@ -578,6 +602,10 @@
 loop 

r281277 - [Sema] Fix PR30346: relax __builtin_object_size checks.

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Mon Sep 12 18:50:35 2016
New Revision: 281277

URL: http://llvm.org/viewvc/llvm-project?rev=281277=rev
Log:
[Sema] Fix PR30346: relax __builtin_object_size checks.

This patch makes us act more conservatively when trying to determine
the objectsize for an array at the end of an object. This is in
response to code like the following:

```
struct sockaddr {
  /* snip */
  char sa_data[14];
};

void foo(const char *s) {
  size_t slen = strlen(s) + 1;
  size_t added_len = slen <= 14 ? 0 : slen - 14;
  struct sockaddr *sa = malloc(sizeof(struct sockaddr) + added_len);
  strcpy(sa->sa_data, s);
  // ...
}
```

`__builtin_object_size(sa->sa_data, 1)` would return 14, when there
could be more than 14 bytes at `sa->sa_data`.

Code like this is apparently not uncommon. FreeBSD's manual even
explicitly mentions this pattern:
https://www.freebsd.org/doc/en/books/developers-handbook/sockets-essential-functions.html
(section 7.5.1.1.2).

In light of this, we now just give up on any array at the end of an
object if we can't find the object's initial allocation.

I lack numbers for how much more conservative we actually become as a
result of this change, so I chose the fix that would make us as
compatible with GCC as possible. If we want to be more aggressive, I'm
happy to consider some kind of whitelist or something instead.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/object-size.c
cfe/trunk/test/CodeGen/pass-object-size.c

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=281277=281276=281277=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 12 18:50:35 2016
@@ -6826,14 +6826,21 @@ static bool tryEvaluateBuiltinObjectSize
   // struct Foo *F = (struct Foo *)malloc(sizeof(struct Foo) + strlen(Bar));
   // strcpy(>c[0], Bar);
   //
-  // So, if we see that we're examining a 1-length (or 0-length) array at the
-  // end of a struct with an unknown base, we give up instead of breaking code
-  // that behaves this way. Note that we only do this when Type=1, because
-  // Type=3 is a lower bound, so answering conservatively is fine.
+  // So, if we see that we're examining an array at the end of a struct with an
+  // unknown base, we give up instead of breaking code that behaves this way.
+  // Note that we only do this when Type=1, because Type=3 is a lower bound, so
+  // answering conservatively is fine.
+  //
+  // We used to be a bit more aggressive here; we'd only be conservative if the
+  // array at the end was flexible, or if it had 0 or 1 elements. This broke
+  // some common standard library extensions (PR30346), but was otherwise
+  // seemingly fine. It may be useful to reintroduce this behavior with some
+  // sort of whitelist. OTOH, it seems that GCC is always conservative with the
+  // last element in structs (if it's an array), so our current behavior is 
more
+  // compatible than a whitelisting approach would be.
   if (End.InvalidBase && SubobjectOnly && Type == 1 &&
   End.Designator.Entries.size() == End.Designator.MostDerivedPathLength &&
   End.Designator.MostDerivedIsArrayElement &&
-  End.Designator.MostDerivedArraySize < 2 &&
   isDesignatorAtObjectEnd(Info.Ctx, End))
 return false;
 

Modified: cfe/trunk/test/CodeGen/object-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=281277=281276=281277=diff
==
--- cfe/trunk/test/CodeGen/object-size.c (original)
+++ cfe/trunk/test/CodeGen/object-size.c Mon Sep 12 18:50:35 2016
@@ -276,7 +276,7 @@ void test23(struct Test23Ty *p) {
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(>t[5], 0);
-  // CHECK: store i32 20
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(>t[5], 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(>t[5], 2);
@@ -444,7 +444,7 @@ void test29(struct DynStructVar *dv, str
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(ss->snd, 0);
-  // CHECK: store i32 2
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(ss->snd, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(ss->snd, 2);
@@ -505,7 +505,7 @@ void test31() {
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(ds1[9].snd, 1);
 
-  // CHECK: store i32 2
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([9].snd[0], 1);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 

r281278 - [DebugInfo] Deduplicate debug info limiting logic

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Sep 12 19:01:23 2016
New Revision: 281278

URL: http://llvm.org/viewvc/llvm-project?rev=281278=rev
Log:
[DebugInfo] Deduplicate debug info limiting logic

We should be doing the same checks when a type is completed as we do
when a complete type is used during emission. Previously, we duplicated
the logic, and it got out of sync. This could be observed with
dllimported classes.

Also reduce a test case for this slightly.

Implementing review feedback from David Blaikie on r281057.

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281278=281277=281278=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Sep 12 19:01:23 2016
@@ -1644,27 +1644,6 @@ void CGDebugInfo::completeType(const Rec
 completeRequiredType(RD);
 }
 
-void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
-  if (DebugKind <= codegenoptions::DebugLineTablesOnly)
-return;
-
-  // If this is a dynamic class and we're emitting limited debug info, wait
-  // until the vtable is emitted to complete the class debug info.
-  if (DebugKind <= codegenoptions::LimitedDebugInfo) {
-if (const auto *CXXDecl = dyn_cast(RD))
-  if (CXXDecl->isDynamicClass())
-return;
-  }
-
-  if (DebugTypeExtRefs && RD->isFromASTFile())
-return;
-
-  QualType Ty = CGM.getContext().getRecordType(RD);
-  llvm::DIType *T = getTypeOrNull(Ty);
-  if (T && T->isForwardDecl())
-completeClassData(RD);
-}
-
 void CGDebugInfo::completeClassData(const RecordDecl *RD) {
   if (DebugKind <= codegenoptions::DebugLineTablesOnly)
 return;
@@ -1763,6 +1742,16 @@ static bool shouldOmitDefinition(codegen
   return false;
 }
 
+void CGDebugInfo::completeRequiredType(const RecordDecl *RD) {
+  if (shouldOmitDefinition(DebugKind, DebugTypeExtRefs, RD, CGM.getLangOpts()))
+return;
+
+  QualType Ty = CGM.getContext().getRecordType(RD);
+  llvm::DIType *T = getTypeOrNull(Ty);
+  if (T && T->isForwardDecl())
+completeClassData(RD);
+}
+
 llvm::DIType *CGDebugInfo::CreateType(const RecordType *Ty) {
   RecordDecl *RD = Ty->getDecl();
   llvm::DIType *T = cast_or_null(getTypeOrNull(QualType(Ty, 0)));

Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281278=281277=281278=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Mon Sep 12 19:01:23 
2016
@@ -6,10 +6,7 @@
 // more general than that.
 
 struct UnicodeString;
-struct GetFwdDecl {
-  static UnicodeString format;
-};
-GetFwdDecl force_fwd_decl;
+UnicodeString *force_fwd_decl;
 struct UnicodeString {
 private:
   virtual ~UnicodeString();

Modified: cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp?rev=281278=281277=281278=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-dllimport-base-class.cpp Mon Sep 12 
19:01:23 2016
@@ -4,6 +4,10 @@
 // be imported from a DLL.  Otherwise, the debugger wouldn't be able to show 
the
 // members.
 
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: 
"ImportedAfterCompletion",
+// CHECK-NOT:  DIFlagFwdDecl
+// CHECK-SAME: ){{$}}
+
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "OutOfLineCtor",
 // CHECK-SAME: DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
@@ -16,6 +20,13 @@
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
 
+
+struct ImportedAfterCompletion;
+ImportedAfterCompletion *force_fwd_decl;
+struct __declspec(dllimport) ImportedAfterCompletion {
+  virtual ~ImportedAfterCompletion();
+};
+
 struct OutOfLineCtor {
   OutOfLineCtor();
   virtual void Foo();
@@ -35,6 +46,7 @@ struct ImportedMethod {
 };
 
 int main() {
+  ImportedAfterCompletion c;
   OutOfLineCtor o;
   DerivedFromImported d;
   ImportedMethod m;


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