Re: [PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC

2016-04-30 Thread Hubert Tong via cfe-commits 
hubert.reinterpretcast updated this revision to Diff 55728.
hubert.reinterpretcast added a comment.

Use AlignOf instead of AlignmentCalcHelper

Since BaseTy should be aligned properly for its trailing objects, the
use of AlignmentCalcHelper is unnecessary.


http://reviews.llvm.org/D19770

Files:
  include/llvm/Support/TrailingObjects.h

Index: include/llvm/Support/TrailingObjects.h
===
--- include/llvm/Support/TrailingObjects.h
+++ include/llvm/Support/TrailingObjects.h
@@ -342,6 +342,37 @@
TrailingTys, size_t>::type... Counts) {
 return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, 
Counts...);
   }
+
+  /// A type where its ::_ template member is suitable for use as
+  /// uninitialized storage of an object if it were allocated with the given
+  /// trailing object counts. The template arguments are similar to those of
+  /// additionalSizeToAlloc.
+  template  struct FixedSizeStorage {
+template 
+using _ =
+llvm::AlignedCharArray;
+  };
+
+  /// A type that acts as the owner for an object placed into fixed storage.
+  class FixedSizeStorageOwner {
+  public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
+~FixedSizeStorageOwner() {
+  assert(p && "FixedSizeStorageOwner owns null?");
+  p->~BaseTy();
+}
+
+BaseTy *get() { return p; }
+
+  private:
+FixedSizeStorageOwner(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner(FixedSizeStorageOwner &&) = delete;
+FixedSizeStorageOwner =(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner =(FixedSizeStorageOwner &&) = delete;
+
+BaseTy *const p;
+  };
 };
 
 } // end namespace llvm


Index: include/llvm/Support/TrailingObjects.h
===
--- include/llvm/Support/TrailingObjects.h
+++ include/llvm/Support/TrailingObjects.h
@@ -342,6 +342,37 @@
TrailingTys, size_t>::type... Counts) {
 return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, Counts...);
   }
+
+  /// A type where its ::_ template member is suitable for use as
+  /// uninitialized storage of an object if it were allocated with the given
+  /// trailing object counts. The template arguments are similar to those of
+  /// additionalSizeToAlloc.
+  template  struct FixedSizeStorage {
+template 
+using _ =
+llvm::AlignedCharArray;
+  };
+
+  /// A type that acts as the owner for an object placed into fixed storage.
+  class FixedSizeStorageOwner {
+  public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
+~FixedSizeStorageOwner() {
+  assert(p && "FixedSizeStorageOwner owns null?");
+  p->~BaseTy();
+}
+
+BaseTy *get() { return p; }
+
+  private:
+FixedSizeStorageOwner(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner(FixedSizeStorageOwner &&) = delete;
+FixedSizeStorageOwner =(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner =(FixedSizeStorageOwner &&) = delete;
+
+BaseTy *const p;
+  };
 };
 
 } // end namespace llvm
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19658: [X86] Add -m[no-]x87 and -m[no-]80387 options to control FeatureX87

2016-04-30 Thread Eric Christopher via cfe-commits 
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Nice. If you wouldn't mind splitting out into 3 patches (all ok):

a) The initial testcase,
b) the mno-cx16 patch and associated test
c) The x87 part and associated test.

You'll also want to put in that there's definitely a bit of "last one wins" on 
the testcases and so that the test case isn't exhaustive.

Thanks!

-eric


http://reviews.llvm.org/D19658



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Peter Collingbourne via cfe-commits 
pcc added a comment.

> It appears that Clang already supports an -mthread_model=single option, so do 
> you recommend that I remove the -mllvm -safe-stack-usp-storage=single-thread 
> option and rely on -mthread_model instead?


Makes sense to me. We're free to break `-mllvm` flags, they are for 
development/debugging purposes only.

Of course, the global variable USP requires runtime support (which I assume 
Contiki provides), so we can probably gate this on OS + thread model.

> I don't see any existing option that seems suitable for indicating that a 
> file may be used during dynamic linker or C library initialization, so 
> perhaps we should add an -mruntime-init flag?


Yes, I'm not sure about the name but we should do something like this.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Michael LeMay via cfe-commits 
mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417865, @pcc wrote:

> > This makes sense for the example I gave. However, there are also more 
> > complicated situations. Sometimes it is necessary to specify different 
> > options for different files that are compiled for the same OS. For example, 
> > early during the initialization of a dynamic linker or C library, a 
> > single-threaded mode of USP storage needs to be supported. TLS is not 
> > available at that time. How should requirements like that be conveyed to 
> > the compiler?
>
>
> In cases like this, we should introduce a new `-m` flag in the Clang driver 
> and plumb that through to the SafeStack pass using a target-specific function 
> attribute.


It appears that Clang already supports an `-mthread_model=single` option, so do 
you recommend that I remove the `-mllvm -safe-stack-usp-storage=single-thread` 
option and rely on `-mthread_model` instead? I added @eugenis to get his 
feedback as well, since he approved my patch that added the new flag.  `-mllvm 
-safe-stack-usp-storage=single-thread` would have been documented for the first 
time by this patch, and I have not heard of anyone else using it yet.

I don't see any existing option that seems suitable for indicating that a file 
may be used during dynamic linker or C library initialization, so perhaps we 
should add an `-mruntime-init` flag?  I welcome other naming suggestions.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC

2016-04-30 Thread Hubert Tong via cfe-commits 
hubert.reinterpretcast added a comment.

An example of using `FixedSizeStorage` can be found in 
http://reviews.llvm.org/D19771.



Comment at: include/llvm/Support/TrailingObjects.h:352
@@ +351,3 @@
+template 
+using _ =
+llvm::AlignedCharArray

[PATCH] D19771: Rework FixedSizeTemplateParameterListStorage

2016-04-30 Thread Hubert Tong via cfe-commits 
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: rsmith, faisalv, aaron.ballman.
hubert.reinterpretcast added subscribers: nwilson, cfe-commits.

This change replaces the custom FixedSizeTemplateParameterListStorage 
implementation with one that follows the interface provided by 
llvm::TrailingObjects.

http://reviews.llvm.org/D19771

Files:
  include/clang/AST/DeclTemplate.h

Index: include/clang/AST/DeclTemplate.h
===
--- include/clang/AST/DeclTemplate.h
+++ include/clang/AST/DeclTemplate.h
@@ -141,31 +141,18 @@
 
 /// \brief Stores a list of template parameters for a TemplateDecl and its
 /// derived classes. Suitable for creating on the stack.
-template  class FixedSizeTemplateParameterListStorage {
-  // This is kinda ugly: TemplateParameterList usually gets allocated
-  // in a block of memory with NamedDecls appended to it. Here, to get
-  // it stack allocated, we include the params as a separate
-  // variable. After allocation, the TemplateParameterList object
-  // treats them as part of itself.
-  TemplateParameterList List;
-  NamedDecl *Params[N];
+template 
+class FixedSizeTemplateParameterListStorage
+: public TemplateParameterList::FixedSizeStorageOwner {
+  TemplateParameterList::FixedSizeStorage::_ storage;
 
 public:
   FixedSizeTemplateParameterListStorage(SourceLocation TemplateLoc,
 SourceLocation LAngleLoc,
 ArrayRef Params,
 SourceLocation RAngleLoc)
-  : List(TemplateLoc, LAngleLoc, Params, RAngleLoc) {
-// Because we're doing an evil layout hack above, have some
-// asserts, just to double-check everything is laid out like
-// expected.
-assert(sizeof(*this) ==
-   TemplateParameterList::totalSizeToAlloc(N) &&
-   "Object layout not as expected");
-assert(this->Params == List.getTrailingObjects() &&
-   "Object layout not as expected");
-  }
-  TemplateParameterList *get() { return  }
+  : FixedSizeStorageOwner(new (static_cast())
+TemplateParameterList(TemplateLoc, LAngleLoc, Params, RAngleLoc)) 
{}
 };
 
 /// \brief A template argument list.


Index: include/clang/AST/DeclTemplate.h
===
--- include/clang/AST/DeclTemplate.h
+++ include/clang/AST/DeclTemplate.h
@@ -141,31 +141,18 @@
 
 /// \brief Stores a list of template parameters for a TemplateDecl and its
 /// derived classes. Suitable for creating on the stack.
-template  class FixedSizeTemplateParameterListStorage {
-  // This is kinda ugly: TemplateParameterList usually gets allocated
-  // in a block of memory with NamedDecls appended to it. Here, to get
-  // it stack allocated, we include the params as a separate
-  // variable. After allocation, the TemplateParameterList object
-  // treats them as part of itself.
-  TemplateParameterList List;
-  NamedDecl *Params[N];
+template 
+class FixedSizeTemplateParameterListStorage
+: public TemplateParameterList::FixedSizeStorageOwner {
+  TemplateParameterList::FixedSizeStorage::_ storage;
 
 public:
   FixedSizeTemplateParameterListStorage(SourceLocation TemplateLoc,
 SourceLocation LAngleLoc,
 ArrayRef Params,
 SourceLocation RAngleLoc)
-  : List(TemplateLoc, LAngleLoc, Params, RAngleLoc) {
-// Because we're doing an evil layout hack above, have some
-// asserts, just to double-check everything is laid out like
-// expected.
-assert(sizeof(*this) ==
-   TemplateParameterList::totalSizeToAlloc(N) &&
-   "Object layout not as expected");
-assert(this->Params == List.getTrailingObjects() &&
-   "Object layout not as expected");
-  }
-  TemplateParameterList *get() { return  }
+  : FixedSizeStorageOwner(new (static_cast())
+TemplateParameterList(TemplateLoc, LAngleLoc, Params, RAngleLoc)) {}
 };
 
 /// \brief A template argument list.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC

2016-04-30 Thread Hubert Tong via cfe-commits 
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: rsmith, faisalv, aaron.ballman.
hubert.reinterpretcast added subscribers: nwilson, cfe-commits, llvm-commits.

This change introduces two types, `FixedSizeStorage` and 
`FixedSizeStorageOwner`, which can be used to provide stack-allocated objects 
with trailing objects.

http://reviews.llvm.org/D19770

Files:
  include/llvm/Support/TrailingObjects.h

Index: include/llvm/Support/TrailingObjects.h
===
--- include/llvm/Support/TrailingObjects.h
+++ include/llvm/Support/TrailingObjects.h
@@ -342,6 +342,38 @@
TrailingTys, size_t>::type... Counts) {
 return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, 
Counts...);
   }
+
+  /// A type where its ::_ template member is suitable for use as
+  /// uninitialized storage of an object if it were allocated with the given
+  /// trailing object counts. The template arguments are similar to those of
+  /// additionalSizeToAlloc.
+  template  struct FixedSizeStorage {
+template 
+using _ =
+llvm::AlignedCharArray::Alignment,
+   totalSizeToAlloc(Counts...)>;
+  };
+
+  /// A type that acts as the owner for an object placed into fixed storage.
+  class FixedSizeStorageOwner {
+  public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
+~FixedSizeStorageOwner() {
+  assert(p && "FixedSizeStorageOwner owns null?");
+  p->~BaseTy();
+}
+
+BaseTy *get() { return p; }
+
+  private:
+FixedSizeStorageOwner(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner(FixedSizeStorageOwner &&) = delete;
+FixedSizeStorageOwner =(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner =(FixedSizeStorageOwner &&) = delete;
+
+BaseTy *const p;
+  };
 };
 
 } // end namespace llvm


Index: include/llvm/Support/TrailingObjects.h
===
--- include/llvm/Support/TrailingObjects.h
+++ include/llvm/Support/TrailingObjects.h
@@ -342,6 +342,38 @@
TrailingTys, size_t>::type... Counts) {
 return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, Counts...);
   }
+
+  /// A type where its ::_ template member is suitable for use as
+  /// uninitialized storage of an object if it were allocated with the given
+  /// trailing object counts. The template arguments are similar to those of
+  /// additionalSizeToAlloc.
+  template  struct FixedSizeStorage {
+template 
+using _ =
+llvm::AlignedCharArray::Alignment,
+   totalSizeToAlloc(Counts...)>;
+  };
+
+  /// A type that acts as the owner for an object placed into fixed storage.
+  class FixedSizeStorageOwner {
+  public:
+FixedSizeStorageOwner(BaseTy *p) : p(p) {}
+~FixedSizeStorageOwner() {
+  assert(p && "FixedSizeStorageOwner owns null?");
+  p->~BaseTy();
+}
+
+BaseTy *get() { return p; }
+
+  private:
+FixedSizeStorageOwner(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner(FixedSizeStorageOwner &&) = delete;
+FixedSizeStorageOwner =(const FixedSizeStorageOwner &) = delete;
+FixedSizeStorageOwner =(FixedSizeStorageOwner &&) = delete;
+
+BaseTy *const p;
+  };
 };
 
 } // end namespace llvm
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Peter Collingbourne via cfe-commits 
pcc added a comment.

> This makes sense for the example I gave. However, there are also more 
> complicated situations. Sometimes it is necessary to specify different 
> options for different files that are compiled for the same OS. For example, 
> early during the initialization of a dynamic linker or C library, a 
> single-threaded mode of USP storage needs to be supported. TLS is not 
> available at that time. How should requirements like that be conveyed to the 
> compiler?


In cases like this, we should introduce a new `-m` flag in the Clang driver and 
plumb that through to the SafeStack pass using a target-specific function 
attribute.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Michael LeMay via cfe-commits 
mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417857, @pcc wrote:

> You should be using `-target x86-unknown-contiki` or similar. That should 
> tune the behaviour to what is required for that OS. See what we have in 
> `TargetLoweringBase::getSafeStackPointerLocation` to provide Android-specific 
> behaviour for example.


This makes sense for the example I gave.  However, there are also more 
complicated situations.  Sometimes it is necessary to specify different options 
for different files that are compiled for the same OS.  For example, early 
during the initialization of a dynamic linker or C library, a single-threaded 
mode of USP storage needs to be supported.  TLS is not available at that time.  
How should requirements like that be conveyed to the compiler?

> The existence of flags is not justification to add more. Besides, it appears 
> that the `-safe-stack-usp-storage` flag was added without proper review. It 
> was reviewed in http://reviews.llvm.org/D15673, but the mailing list was not 
> added as a subscriber. If I had been aware of that review I would have made 
> the same objections at that time.


Sorry, I only recently learned that the mailing list should be added as a 
subscriber.  Prior to that, I thought that patches were automatically sent to 
the appropriate mailing lists.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Peter Collingbourne via cfe-commits 
pcc added a comment.

You should be using `-target x86-unknown-contiki` or similar. That should tune 
the behaviour to what is required for that OS. See what we have in 
`TargetLoweringBase::getSafeStackPointerLocation` to provide Android-specific 
behaviour for example.

The existence of flags is not justification to add more. Besides, it appears 
that the `-safe-stack-usp-storage` flag was added without proper review. It was 
reviewed in http://reviews.llvm.org/D15673, but the mailing list was not added 
as a subscriber. If I had been aware of that review I would have made the same 
objections at that time.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D17841: [libclang/python] Add bindings for children of diagnostics

2016-04-30 Thread Saleem Abdulrasool via cfe-commits 
compnerd closed this revision.
compnerd added a comment.

Committed as SVN r268167.  Thanks for the patch!


http://reviews.llvm.org/D17841



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

r268167 - python: add bindings for children of diagnostics

2016-04-30 Thread Saleem Abdulrasool via cfe-commits 
Author: compnerd
Date: Sat Apr 30 16:14:01 2016
New Revision: 268167

URL: http://llvm.org/viewvc/llvm-project?rev=268167=rev
Log:
python: add bindings for children of diagnostics

This exposes the Clang API bindings clang_getChildDiagnostics (which returns a
CXDiagnosticSet) and clang_getNumDiagnosticsInSet / clang_getDiagnosticInSet (to
traverse the CXDiagnosticSet), and adds a helper children property in the Python
Diagnostic wrapper.

Also, this adds the missing OVERLOAD_CANDIDATE (700) cursor type.

Patch by Hanson Wang!

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=268167=268166=268167=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Sat Apr 30 16:14:01 2016
@@ -360,6 +360,23 @@ class Diagnostic(object):
 return FixItIterator(self)
 
 @property
+def children(self):
+class ChildDiagnosticsIterator:
+def __init__(self, diag):
+self.diag_set = conf.lib.clang_getChildDiagnostics(diag)
+
+def __len__(self):
+return 
int(conf.lib.clang_getNumDiagnosticsInSet(self.diag_set))
+
+def __getitem__(self, key):
+diag = conf.lib.clang_getDiagnosticInSet(self.diag_set, key)
+if not diag:
+raise IndexError
+return Diagnostic(diag)
+
+return ChildDiagnosticsIterator(self)
+
+@property
 def category_number(self):
 """The category number for this diagnostic or 0 if unavailable."""
 return conf.lib.clang_getDiagnosticCategory(self)
@@ -1120,6 +1137,9 @@ CursorKind.MODULE_IMPORT_DECL = CursorKi
 # A type alias template declaration
 CursorKind.TYPE_ALIAS_TEMPLATE_DECL = CursorKind(601)
 
+# A code completion overload candidate.
+CursorKind.OVERLOAD_CANDIDATE = CursorKind(700)
+
 ### Template Argument Kinds ###
 class TemplateArgumentKind(BaseEnumeration):
 """
@@ -3053,6 +3073,10 @@ functionList = [
Type,
Type.from_result),
 
+  ("clang_getChildDiagnostics",
+   [Diagnostic],
+   c_object_p),
+
   ("clang_getCompletionAvailability",
[c_void_p],
c_int),
@@ -3173,6 +3197,10 @@ functionList = [
_CXString,
_CXString.from_result),
 
+  ("clang_getDiagnosticInSet",
+   [c_object_p, c_uint],
+   c_object_p),
+
   ("clang_getDiagnosticLocation",
[Diagnostic],
SourceLocation),
@@ -3274,6 +3302,10 @@ functionList = [
[c_object_p],
c_uint),
 
+  ("clang_getNumDiagnosticsInSet",
+   [c_object_p],
+   c_uint),
+
   ("clang_getNumElements",
[Type],
c_longlong),

Modified: cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py?rev=268167=268166=268167=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py Sat Apr 30 
16:14:01 2016
@@ -80,3 +80,15 @@ def test_diagnostic_option():
 
 assert d.option == '-Wunused-parameter'
 assert d.disable_option == '-Wno-unused-parameter'
+
+def test_diagnostic_children():
+tu = get_tu('void f(int x) {} void g() { f(); }')
+assert len(tu.diagnostics) == 1
+d = tu.diagnostics[0]
+
+children = d.children
+assert len(children) == 1
+assert children[0].severity == Diagnostic.Note
+assert children[0].spelling.endswith('declared here')
+assert children[0].location.line == 1
+assert children[0].location.column == 1


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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Michael LeMay via cfe-commits 
mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417844, @pcc wrote:

> We shouldn't be adding (much less documenting) `-mllvm` flags. Is there any 
> reason why this behavior can't be gated on the OS?


We actually already have added at least one `-mllvm` flag: `-mllvm 
-safe-stack-usp-storage=single-thread`.  You can see an example of how it can 
be used here: 
https://github.com/contiki-os/contiki/pull/1642/commits/f13d1e22669d5526f2a721a337acd06f23a8d49d
  Can you please provide an example of how you think that setting should be 
specified without the use of the `-mllvm` flag?


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

2016-04-30 Thread Peter Collingbourne via cfe-commits 
pcc added a comment.

We shouldn't be adding (much less documenting) `-mllvm` flags. Is there any 
reason why this behavior can't be gated on the OS?


http://reviews.llvm.org/D19483



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

Re: [PATCH] RHEL: Look in more places to find g++ headers and runtime

2016-04-30 Thread Rafael Espíndola via cfe-commits 
LGTM. Do you have commit access?

Cheers,
Rafael


On 11 April 2016 at 14:49, Michael Lampe  wrote:
> New patch attached.
>
> -Michael
>
>
> Rafael Espíndola wrote:
>>
>> LGTM with a comment saying why it is needed.
>>
>> Cheers,
>> Rafael
>>
>>
>> On 22 March 2016 at 23:02, Michael Lampe via cfe-commits
>>  wrote:
>>>
>>> Some distros with ten years of support ship an old gcc but later offer
>>> more
>>> recent versions for installation in parallel. These versions are
>>> typically
>>> not only needed for the compilation of llvm/clang, but also to properly
>>> use
>>> the clang binary that comes out.
>>>
>>> Clang already searches /usr at runtime for the most recent installation
>>> of
>>> gcc. This patch appends paths for add-on installations of gcc in RHEL.
>>>
>>>
>>> ___
>>> 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] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.

2016-04-30 Thread Etienne Bergeron via cfe-commits 
etienneb added a comment.

If you are interested to a quick chat on this Checker, ping me. I know other 
cases that should be filtered and I'm lacking time to implement them. Here is a 
common one I would lie to see happening (base on comments):

  const char* A[] = {
// This is a entry
"entry1",
// This is the next entry
"entry2"
"entry2"
"entry2",
// This is the last entry
"entry2",
  };

Other common cases to append literals are:

  "Program version:"  VERSION 
  "Program version: %"  PRIu64 
  "Let add strange characters \xFF" "For nothing"  (you can't happend these two 
literals or the escaped char will be "\xFFF")



Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:92
@@ +91,3 @@
+  has(expr(ignoringImpCasts(ConcatenatedStringLiteral))),
+  hasParent(varDecl(allOf(hasType(arrayType()), isDefinition()))
+.bind("varDecl")));

The issue I see by matching the "hasParent" that way is that you are assuming 
that the matcher is only looking for initialisation-list attached to variable 
declaration (direct child).

Assume this case:
```
struct StrArray {
  int n;
  const char* list[5];
} A[2] = {
  {5, {"a", "b", "c", "d", "e" }},
  {5, {"a", "b", "c", "d", "e" }},
};
```

It is giving you this AST representation:
```
VarDecl 0x1e82738  line:6:3 A 'struct StrArray [2]' cinit
`-InitListExpr 0x1e82c38  'struct StrArray [2]'
  |-InitListExpr 0x1e82c88  'struct StrArray':'struct 
StrArray'
  | |-IntegerLiteral 0x1e827d8  'int' 5
  | `-InitListExpr 0x1e82cd8  'const char *[5]'
  |   |-ImplicitCastExpr 0x1e82d40  'const char *' 
  |   | `-StringLiteral 0x1e82878  'const char [2]' lvalue "a"
  |   |-ImplicitCastExpr 0x1e82d58  'const char *' 
  |   | `-StringLiteral 0x1e828a8  'const char [2]' lvalue "b"
  |   |-ImplicitCastExpr 0x1e82d70  'const char *' 
  |   | `-StringLiteral 0x1e828d8  'const char [2]' lvalue "c"
  |   |-ImplicitCastExpr 0x1e82d88  'const char *' 
  |   | `-StringLiteral 0x1e82908  'const char [2]' lvalue "d"
  |   `-ImplicitCastExpr 0x1e82da0  'const char *' 
  | `-StringLiteral 0x1e82938  'const char [2]' lvalue "e"
  `-InitListExpr 0x1e82db8  'struct StrArray':'struct 
StrArray'
|-IntegerLiteral 0x1e82a20  'int' 5
`-InitListExpr 0x1e82e08  'const char *[5]'
  |-ImplicitCastExpr 0x1e82e70  'const char *' 
  | `-StringLiteral 0x1e82a40  'const char [2]' lvalue "a"
  |-ImplicitCastExpr 0x1e82e88  'const char *' 
  | `-StringLiteral 0x1e82a70  'const char [2]' lvalue "b"
  |-ImplicitCastExpr 0x1e82ea0  'const char *' 
  | `-StringLiteral 0x1e82aa0  'const char [2]' lvalue "c"
  |-ImplicitCastExpr 0x1e82eb8  'const char *' 
  | `-StringLiteral 0x1e82ad0  'const char [2]' lvalue "d"
  `-ImplicitCastExpr 0x1e82ed0  'const char *' 
`-StringLiteral 0x1e82b00  'const char [2]' lvalue "e"
```

I believe this can be solved with something like:
  hasParent(anyOf(varDecl(allOf(hasType(arrayType()), isDefinition()),
  anything()))  <<-- to accept all other cases.

That way, you are adding an heuristic to filter some incorrect warnings.

I believe it's possible to match the example above as the size is part of the 
type.

If I try this example:

```
{5, {"a", "b", "c", "d"}},// only four string literal
```

I'm getting the following AST:
```
 `-InitListExpr 0x28ffd50  'struct StrArray':'struct StrArray'
|-IntegerLiteral 0x28ff9e8  'int' 5
`-InitListExpr 0x28ffda0  'const char *[5]'
  |-array filler
  | `-ImplicitValueInitExpr 0x28ffe78 <> 'const char *'
  |-ImplicitCastExpr 0x28ffde0  'const char *' 
  | `-StringLiteral 0x28ffa08  'const char [2]' lvalue "a"
  |-ImplicitCastExpr 0x28ffe00  'const char *' 
  | `-StringLiteral 0x28ffa38  'const char [2]' lvalue "b"
  |-ImplicitCastExpr 0x28ffe28  'const char *' 
  | `-StringLiteral 0x28ffa68  'const char [2]' lvalue "c"
  `-ImplicitCastExpr 0x28ffe60  'const char *' 
`-StringLiteral 0x28ffa98  'const char [2]' lvalue "d"
```

For the direct case:
```
const char* list[5] = { "a", "b"};
```

It has the following AST-representation:

```
VarDecl 0x2c67f00  col:13 list 'const char *[5]' cinit
`-InitListExpr 0x2c68010  'const char *[5]'
  |-array filler
  | `-ImplicitValueInitExpr 0x2c68098 <> 'const char *'
  |-ImplicitCastExpr 0x2c68050  'const char *' 
  | `-StringLiteral 0x2c67f60  'const char [2]' lvalue "a"
  `-ImplicitCastExpr 0x2c68070  'const char *' 
`-StringLiteral 0x2c67f90  'const char [2]' lvalue "b"
```
So, it seems the length could be taken from the type instead of the 
declaration?!
Or, look at what is the "Array Filler" (I still don't know). This may be an 
more straight forward way.




http://reviews.llvm.org/D19769

[PATCH] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.

2016-04-30 Thread Dominik Szabó via cfe-commits 
szdominik created this revision.
szdominik added reviewers: alexfh, etienneb.
szdominik added subscribers: cfe-commits, xazax.hun.

Additional heuristic to misc-suspicious-missing-comma checker, based on the 
(in)equality of the explicitly given array size and the real array size.
Note: in these cases we don't know that the given size is wrong or there is a 
missing comma.

Original checker revision: http://reviews.llvm.org/D18457

http://reviews.llvm.org/D19769

Files:
  clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
  docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
  test/clang-tidy/misc-suspicious-missing-comma.cpp

Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -80,3 +80,21 @@
   "Dummy line",
   "Dummy line",
 };
+
+// Missing comma or wrong explicit array size.
+const char* TheThreeMusketeers[4] = {
+  "Athos",
+  "Porthos",
+  "Aramis"
+  "D'Artagnan"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: wrong string array initialization: 
the explicit given size and the real number of elements aren't equal 
[misc-suspicious-missing-comma]
+
+// Correctly given array size should avoid warning.
+const char* TheFourMusketeers[4] = {
+  "Athos",
+  "Porthos",
+  "Aramis",
+  "Charles de Batz de Castelmore"
+  "D'Artagnan"
+};
Index: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
===
--- docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
+++ docs/clang-tidy/checks/misc-suspicious-missing-comma.rst
@@ -42,3 +42,14 @@
 "Warning %s",
   };
 
+This checker is also capable of warn on cases when the explicitly given array 
size
+isn't equal to the real array size.
+
+.. code:: c++
+
+  const char* TheThreeMusketeers[4] = {
+  "Athos",
+  "Porthos",
+  "Aramis"
+  "D'Artagnan"
+};
Index: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
===
--- clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
+++ clang-tidy/misc/SuspiciousMissingCommaCheck.cpp
@@ -86,9 +86,11 @@
   const auto ConcatenatedStringLiteral =
   stringLiteral(isConcatenatedLiteral(MaxConcatenatedTokens)).bind("str");
 
-  const auto StringsInitializerList =
-  initListExpr(hasType(constantArrayType()),
-   has(expr(ignoringImpCasts(ConcatenatedStringLiteral;
+  const auto StringsInitializerList = initListExpr(
+  hasType(constantArrayType()),
+  has(expr(ignoringImpCasts(ConcatenatedStringLiteral))),
+  hasParent(varDecl(allOf(hasType(arrayType()), isDefinition()))
+.bind("varDecl")));
 
   Finder->addMatcher(StringsInitializerList.bind("list"), this);
 }
@@ -98,10 +100,29 @@
   const auto *InitializerList = Result.Nodes.getNodeAs("list");
   const auto *ConcatenatedLiteral =
   Result.Nodes.getNodeAs("str");
-  assert(InitializerList && ConcatenatedLiteral);
-  
-  // Skip small arrays as they often generate false-positive.
+  const auto *VariableDeclaration = Result.Nodes.getNodeAs("varDecl");
+  assert(InitializerList && ConcatenatedLiteral && VariableDeclaration);
+
   unsigned int Size = InitializerList->getNumInits();
+
+  // Warn when the explicit given array size isn't equal to the real array 
size.
+  QualType DeclType = VariableDeclaration->getTypeSourceInfo()->getType();
+  if (DeclType->isConstantArrayType()) {
+unsigned int ExplicitArraySize = 0;
+ExplicitArraySize =
+((Result.Context)->getAsConstantArrayType(DeclType)->getSize())
+.getLimitedValue();
+
+if (Size != ExplicitArraySize) {
+  diag(InitializerList->getExprLoc(),
+   "wrong string array initialization: "
+   "the explicit given size and the "
+   "real number of elements aren't equal");
+  return;
+}
+  }
+
+  // Skip small arrays as they often generate false-positive.
   if (Size < SizeThreshold) return;
 
   // Count the number of occurence of concatenated string literal.


Index: test/clang-tidy/misc-suspicious-missing-comma.cpp
===
--- test/clang-tidy/misc-suspicious-missing-comma.cpp
+++ test/clang-tidy/misc-suspicious-missing-comma.cpp
@@ -80,3 +80,21 @@
   "Dummy line",
   "Dummy line",
 };
+
+// Missing comma or wrong explicit array size.
+const char* TheThreeMusketeers[4] = {
+  "Athos",
+  "Porthos",
+  "Aramis"
+  "D'Artagnan"
+};
+// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: wrong string array initialization: the explicit given size and the real number of elements aren't equal [misc-suspicious-missing-comma]
+
+// Correctly given array size should avoid warning.
+const char* TheFourMusketeers[4] = {
+  "Athos",
+  "Porthos",
+  "Aramis",
+  "Charles de Batz de Castelmore"
+  "D'Artagnan"
+};
Index:

Re: [PATCH] D18073: Add memory allocating functions

2016-04-30 Thread Anna Zaks via cfe-commits 
zaks.anna added a comment.

You conditionally defined when you build ON Windows machine, not when you build 
FOR Windows. You should be able to query the compiler to check which targets 
it's building for.


http://reviews.llvm.org/D18073



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

Re: [PATCH] D19312: Warn about UB at member function calls from base class ctor initializers.

2016-04-30 Thread Raphael Isemann via cfe-commits 
teemperor updated this revision to Diff 55716.
teemperor added a comment.

Merged all tests into one file.


http://reviews.llvm.org/D19312

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/warn-undefined-in-ctor.cpp

Index: test/SemaCXX/warn-undefined-in-ctor.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-undefined-in-ctor.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wundefined-call-from-ctor
+
+// Warn about member function calls
+namespace MemberFunctionCallsTests {
+
+  // Helper class for the following test cases.
+  class A {
+  public:
+A(int i) {}
+  };
+
+  // Calling member functions before bass class initialized is undefined behavior.
+  class B : public A {
+int member_;
+
+  public:
+B()
+: A(1 + get_i()) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+}
+B(float var)
+: A(sizeof(get_i())) { // no-warning
+}
+B(int var) : A(0), member_(get_i()) {} // no-warning
+
+int get_i() { return 2; }
+  };
+
+  // Same as previous test but with explicit this.
+  class C : public A {
+  public:
+C()
+: A(this->get_i() + 1) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+}
+
+int get_i() { return 2; }
+  };
+
+  // Check that the whole ctor-initializer is checked for member calls.
+  class OtherA {
+  public:
+OtherA(int i) {}
+  };
+
+  class D : public OtherA, public A {
+  public:
+D()
+: OtherA(this->get_i() + 1), A(this->get_i() + 1) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'OtherA' results in undefined behavior}} \
+// expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+}
+
+int get_i() { return 2; }
+  };
+
+  // Calling static functions of this class is not undefined behavior.
+  class E : public A {
+  public:
+E() : A(this->get_i() + 1) { // no-warning
+}
+
+static int get_i() { return 2; }
+  };
+
+  // Calling other functions of this class is not undefined behavior.
+  int other_foo() { return 2; }
+  class F : public A {
+  public:
+F() : A(other_foo()) {} // no-warning
+  };
+
+  // Calling member functions of other classes is not undefined behavior.
+  class G : public A {
+  public:
+G(B ) : A(b.get_i()) {} // no-warning
+  };
+}
+
+
+
+// Warn about member function calls - example code from [C++11 12.6.2 p13]
+namespace MemberFunctionCallsStdTests {
+
+  class A {
+  public:
+A(int);
+  };
+
+  class B : public A {
+int j;
+
+  public:
+int f();
+B()
+: A(f()),   // expected-warning {{member function call this->f() in ctor-initializer for base class 'A' results in undefined behavior}}
+  j(f()) {} // no-warning
+  };
+
+  class C {
+  public:
+C(int);
+  };
+  class D : public B, C {
+int i;
+
+  public:
+D()
+: C(f()),   // expected-warning {{member function call this->f() in ctor-initializer for base class 'C' results in undefined behavior}}
+  i(f()) {} // no-warning
+  };
+
+}
+
+
+
+
+// Warn about dynamic_cast on this
+namespace DynamicCastTests {
+
+  // Helper class for the following test case.
+  class A {
+  public:
+A(A *a) {}
+A(unsigned long a) {}
+  };
+
+  // Calling dynamic cast on this before base class is initialized is undefined
+  // behavior.
+  class B : public A {
+
+A *j;
+
+  public:
+B()
+: A(dynamic_cast(this) + 1), j(nullptr) { // expected-warning {{dynamic_cast with this as operand in ctor-initializer for base class 'A' results in undefined behavior}}
+}
+B(float var)
+: A(sizeof(dynamic_cast(this) + 1)), j(nullptr) { // no-warning
+}
+B(int var) : A(nullptr), j(dynamic_cast(this)) {} // no-warning
+  };
+}
+
+
+
+// Warn about typeid on this
+namespace std {
+  class type_info {
+  };
+}
+
+namespace TypeidTests {
+
+  class A {
+  public:
+A(const std::type_info& a) {}
+A(unsigned a) {}
+  };
+
+  // Calling dynamic cast on this before base class is initialized is undefined
+  // behavior.
+  class B : public A {
+
+const std::type_info& j;
+
+  public:
+B()
+: A(typeid(this)), j(typeid(nullptr)) { // expected-warning {{typeid with this as operand in ctor-initializer for base class 'A' results in undefined behavior}}
+}
+B(float var)
+: A(sizeof(typeid(this))), j(typeid(nullptr)) { // no-warning
+}
+B(int var) : A(typeid(nullptr)), j(typeid(this)) {} // no-warning
+  };
+
+}
Index: lib/Sema/SemaDeclCXX.cpp

Re: [PATCH] D19327: Keep invalid function body as part of the AST

2016-04-30 Thread Olivier Goffart via cfe-commits 
ogoffart added a comment.

An alternative patch is uploaded there:  http://reviews.llvm.org/D19764


http://reviews.llvm.org/D19327



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

[PATCH] D19764: Keep invalid functions as part of the AST

2016-04-30 Thread Olivier Goffart via cfe-commits 
ogoffart created this revision.
ogoffart added reviewers: cfe-commits, rsmith.

This is an alternative to http://reviews.llvm.org/D19327
We want to keep all the invalid function declarations as part of the AST.

This patch depends also on http://reviews.llvm.org/D19763 otherwise the tests 
in SemaCXX/function)redecl.cxx would fail as some invalid  declarations would 
be hiding previous ones, preventing errors

http://reviews.llvm.org/D19764

Files:
  lib/Sema/SemaDecl.cpp
  test/Misc/ast-dump-invalid.cpp
  test/Sema/predefined-function.c

Index: test/Sema/predefined-function.c
===
--- test/Sema/predefined-function.c
+++ test/Sema/predefined-function.c
@@ -4,14 +4,13 @@
 enum Test {A=-1};
 char *funk(enum Test x);
 
-int eli(float b); // expected-note {{previous declaration is here}} \
-// expected-note{{passing argument to parameter 'b' here}}
+int eli(float b); // expected-note {{previous declaration is here}}
 int b(int c) {return 1;}
 
 int foo();
 int foo() {
   int eli(int (int)); // expected-error {{conflicting types for 'eli'}}
-  eli(b); // expected-error{{passing 'int (int)' to parameter of incompatible 
type 'float'}}
+  eli(b);
   return 0;
 }
 
Index: test/Misc/ast-dump-invalid.cpp
===
--- test/Misc/ast-dump-invalid.cpp
+++ test/Misc/ast-dump-invalid.cpp
@@ -41,3 +41,24 @@
 // CHECK-NEXT: `-ImplicitCastExpr {{.*}}  'int' 

 // CHECK-NEXT:   `-DeclRefExpr {{.*}}  'int' lvalue ParmVar 
{{.*}} 'i' 'int'
 
+
+namespace TestInvalidFunctionDecl {
+struct Str {
+   double foo1(double, invalid_type);
+};
+double Str::foo1(double, invalid_type)
+{ return 45; }
+}
+// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidFunctionDecl
+// CHECK-NEXT: |-CXXRecordDecl {{.*}}  line:46:8 struct 
Str definition
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}}  col:8 implicit struct 
Str
+// CHECK-NEXT: | `-CXXMethodDecl {{.*}}  col:11 invalid 
foo1 'double (double, int)'
+// CHECK-NEXT: |   |-ParmVarDecl {{.*}}  col:22 'double'
+// CHECK-NEXT: |   `-ParmVarDecl {{.*}}  col:36 
invalid 'int'
+// CHECK-NEXT: `-CXXMethodDecl {{.*}} parent {{.*}}  
line:49:13 invalid foo1 'double (double, int)'
+// CHECK-NEXT:   |-ParmVarDecl {{.*}}  col:24 'double'
+// CHECK-NEXT:   |-ParmVarDecl {{.*}}  col:38 invalid 
'int'
+// CHECK-NEXT:   `-CompoundStmt {{.*}} 
+// CHECK-NEXT: `-ReturnStmt {{.*}} 
+// CHECK-NEXT:   `-ImplicitCastExpr {{.*}}  'double' 

+// CHECK-NEXT: `-IntegerLiteral {{.*}}  'int' 45
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5049,10 +5049,9 @@
   if (!New)
 return nullptr;
 
-  // If this has an identifier and is not an invalid redeclaration or 
-  // function template specialization, add it to the scope stack.
-  if (New->getDeclName() && AddToScope &&
-   !(D.isRedeclaration() && New->isInvalidDecl())) {
+  // If this has an identifier and is not a function template specialization,
+  // add it to the scope stack.
+  if (New->getDeclName() && AddToScope) {
 // Only make a locally-scoped extern declaration visible if it is the first
 // declaration of this entity. Qualified lookup for such an entity should
 // only find this declaration if there is no visible declaration of it.


Index: test/Sema/predefined-function.c
===
--- test/Sema/predefined-function.c
+++ test/Sema/predefined-function.c
@@ -4,14 +4,13 @@
 enum Test {A=-1};
 char *funk(enum Test x);
 
-int eli(float b); // expected-note {{previous declaration is here}} \
-// expected-note{{passing argument to parameter 'b' here}}
+int eli(float b); // expected-note {{previous declaration is here}}
 int b(int c) {return 1;}
 
 int foo();
 int foo() {
   int eli(int (int)); // expected-error {{conflicting types for 'eli'}}
-  eli(b); // expected-error{{passing 'int (int)' to parameter of incompatible type 'float'}}
+  eli(b);
   return 0;
 }
 
Index: test/Misc/ast-dump-invalid.cpp
===
--- test/Misc/ast-dump-invalid.cpp
+++ test/Misc/ast-dump-invalid.cpp
@@ -41,3 +41,24 @@
 // CHECK-NEXT: `-ImplicitCastExpr {{.*}}  'int' 
 // CHECK-NEXT:   `-DeclRefExpr {{.*}}  'int' lvalue ParmVar {{.*}} 'i' 'int'
 
+
+namespace TestInvalidFunctionDecl {
+struct Str {
+   double foo1(double, invalid_type);
+};
+double Str::foo1(double, invalid_type)
+{ return 45; }
+}
+// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidFunctionDecl
+// CHECK-NEXT: |-CXXRecordDecl {{.*}}  line:46:8 struct Str definition
+// CHECK-NEXT: | |-CXXRecordDecl {{.*}}  col:8 implicit struct Str
+//

[PATCH] D19763: Functions declared in a scope should not hide previous declaration in earlier scopes

2016-04-30 Thread Olivier Goffart via cfe-commits 
ogoffart created this revision.
ogoffart added reviewers: cfe-commits, rsmith.

This code should be an error:

  void foo(int); 
  void f3() {
int foo(float);
{
  float foo(int); // expected-error {{functions that differ only in their 
return type cannot be overloaded}}
}
  }

the foo(float) function declared at function scope should not hide the 
float(int) while trying to redeclare functions.

http://reviews.llvm.org/D19763

Files:
  lib/Sema/SemaLookup.cpp
  test/SemaCXX/function-redecl.cpp

Index: test/SemaCXX/function-redecl.cpp
===
--- test/SemaCXX/function-redecl.cpp
+++ test/SemaCXX/function-redecl.cpp
@@ -7,7 +7,7 @@
 void bar(int); // expected-note 2{{previous declaration is here}}
   }
 
-  void foo(int); // expected-note 2{{previous declaration is here}}
+  void foo(int); // expected-note 3{{previous declaration is here}}
 
   void f2() {
 int foo(int); // expected-error {{functions that differ only in their 
return type cannot be overloaded}}
@@ -25,6 +25,13 @@
   }
 }
   }
+
+  void f3() {
+int foo(float);
+{
+  float foo(int); // expected-error {{functions that differ only in their 
return type cannot be overloaded}}
+}
+  }
 }
 
 class A {
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -1091,23 +1091,25 @@
 bool Found = false;
 for (; I != IEnd && S->isDeclScope(*I); ++I) {
   if (NamedDecl *ND = R.getAcceptableDecl(*I)) {
-if (NameKind == LookupRedeclarationWithLinkage) {
-  // Determine whether this (or a previous) declaration is
-  // out-of-scope.
-  if (!LeftStartingScope && !Initial->isDeclScope(*I))
-LeftStartingScope = true;
-
-  // If we found something outside of our starting scope that
-  // does not have linkage, skip it. If it's a template parameter,
+if (NameKind == LookupRedeclarationWithLinkage &&
+!(*I)->isTemplateParameter()) {
+  // If it's a template parameter,
   // we still find it, so we can diagnose the invalid redeclaration.
-  if (LeftStartingScope && !((*I)->hasLinkage()) &&
-  !(*I)->isTemplateParameter()) {
+
+  // Determine whether this (or a previous) declaration is
+  // out-of-scope.
+  if (!LeftStartingScope && !Initial->isDeclScope(*I))
+LeftStartingScope = true;
+
+  // If we found something outside of our starting scope that
+  // does not have linkage, skip it.
+  if (LeftStartingScope && !((*I)->hasLinkage())) {
 R.setShadowed();
 continue;
   }
+} else {
+  Found = true;
 }
-
-Found = true;
 R.addDecl(ND);
   }
 }


Index: test/SemaCXX/function-redecl.cpp
===
--- test/SemaCXX/function-redecl.cpp
+++ test/SemaCXX/function-redecl.cpp
@@ -7,7 +7,7 @@
 void bar(int); // expected-note 2{{previous declaration is here}}
   }
 
-  void foo(int); // expected-note 2{{previous declaration is here}}
+  void foo(int); // expected-note 3{{previous declaration is here}}
 
   void f2() {
 int foo(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
@@ -25,6 +25,13 @@
   }
 }
   }
+
+  void f3() {
+int foo(float);
+{
+  float foo(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
+}
+  }
 }
 
 class A {
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -1091,23 +1091,25 @@
 bool Found = false;
 for (; I != IEnd && S->isDeclScope(*I); ++I) {
   if (NamedDecl *ND = R.getAcceptableDecl(*I)) {
-if (NameKind == LookupRedeclarationWithLinkage) {
-  // Determine whether this (or a previous) declaration is
-  // out-of-scope.
-  if (!LeftStartingScope && !Initial->isDeclScope(*I))
-LeftStartingScope = true;
-
-  // If we found something outside of our starting scope that
-  // does not have linkage, skip it. If it's a template parameter,
+if (NameKind == LookupRedeclarationWithLinkage &&
+!(*I)->isTemplateParameter()) {
+  // If it's a template parameter,
   // we still find it, so we can diagnose the invalid redeclaration.
-  if (LeftStartingScope && !((*I)->hasLinkage()) &&
-  !(*I)->isTemplateParameter()) {
+
+  // Determine whether this (or a previous) declaration is
+  // out-of-scope.
+  if (!LeftStartingScope && !Initial->isDeclScope(*I))
+LeftStartingScope = true;
+
+  // If we found something outside of our starting scope that

Re: [PATCH] D19654: PR27132: Proper mangling for __unaligned qualifier (now with PR27367 fixed)

2016-04-30 Thread David Majnemer via cfe-commits 
majnemer added inline comments.


Comment at: lib/AST/MicrosoftMangle.cpp:1583-1584
@@ -1579,2 +1582,4 @@
   case QMM_Result:
+// Presence of __unaligned qualifier shouldn't affect mangling here.
+Quals.removeUnaligned();
 if ((!IsPointer && Quals) || isa(T)) {

andreybokhanko wrote:
> Done. Test added.
Hmm, can you give a concrete example why we need this line?


http://reviews.llvm.org/D19654



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

Re: [PATCH] D19412: [libcxx] Refactor pthread usage - II

2016-04-30 Thread Asiri Rathnayake via cfe-commits 
rmaprath added a comment.

In http://reviews.llvm.org/D19412#417682, @EricWF wrote:

> In http://reviews.llvm.org/D19412#416596, @rmaprath wrote:
>
> > In http://reviews.llvm.org/D19412#416183, @EricWF wrote:
> >
> > > OK. I *think* this is my last round of review comments except for one 
> > > specific issue. I'm still looking into the changes to the static mutex's 
> > > and condition_variables in `memory.cpp` and `algorithm.cpp`. In these 
> > > cases I don't want to go from compile-time initialization to run-time 
> > > initialization.  This could introduce the static initialization order 
> > > fiasco.
> >
> >
> > So, as pointed out earlier by @bcraig, there won't be a runtime overhead 
> > for those compilers supporting `constexr`. For those that don't, this is 
> > still a trivial enough constructor call that is very likely to get 
> > optimized away. To elaborate, we can simplify this scenario to something 
> > like:
> >
> > **thread.h**
> >
> >   struct thread_t {
> > int state1;
> > int *state2;
> > bool state3;
> >   };
> >  
> >   #define THREAD_INITIALIZER  {0, 0, false}
> >
> >
> > **test.cpp**
> >
> >   #include "thread.h"
> >  
> >   class Foo {
> >   private:
> > thread_t __t_;
> >   public:
> > Foo() {__t_ = (thread_t) THREAD_INITIALIZER;}
> >   };
> >
>
>
> Foo has a trivial destructor. Our mutex does not. I've already looked into 
> the codegen for clang and in each case your change creates runtime code. The 
> constructors may still be optimized away but it still has to register the 
> destructors.
>
> I'm just trying to figure out if that's going to be a problem.
>
> Example here: https://godbolt.org/g/dJL29v


Hi Eric,

Thanks for the pointer (and also for godbolt!). I can see that that my code 
creates runtime code to register the destructors.

Apart from the overhead (registering the destructors at load time and then 
actually calling them when the DSO is unloaded), I can't see how this could 
lead to other issues. The ABI seem to be have a pretty well defined DSO 
destruction process [1].

Having to handle proper destruction of internal mutexes and condition variables 
sound like an OK thing for a standard library to do. But then, the following 
commit (which replaced `mutex` with the pthread native mutexes in `memory.cpp` 
originally) makes me think twice:

  commit e33c2d1926f49221c9d72a353d797d135a810d77
  Author: Howard Hinnant 
  Date:   Sat Mar 16 00:17:53 2013 +
  
  This should be nothing but a load-time optimization.  I'm trying to 
reduce load time initializers and this is a big one.  No visible functionality 
change intended.
  
  git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@177212 
91177308-0d34-0410-b5e6-96231b3b80d8
  
  diff --git a/src/memory.cpp b/src/memory.cpp
  index 14084a5..98bcc86 100644
  --- a/src/memory.cpp
  +++ b/src/memory.cpp
  @@ -122,7 +122,15 @@ __shared_weak_count::__get_deleter(const type_info&) 
const _NOEXCEPT
   #if __has_feature(cxx_atomic)
   
   static const std::size_t __sp_mut_count = 16;
  -static mutex mut_back[__sp_mut_count];
  +static pthread_mutex_t mut_back_imp[__sp_mut_count] =
  +{
  +PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, 
PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
  +PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, 
PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
  +PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, 
PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER,
  +PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER, 
PTHREAD_MUTEX_INITIALIZER, PTHREAD_MUTEX_INITIALIZER
  +};
  +
  +static mutex* mut_back = reinterpret_cast(mut_back_imp);
   
   _LIBCPP_CONSTEXPR __sp_mut::__sp_mut(void* p) _NOEXCEPT
  : __lx(p)

So, perhaps it is best to leave these pthread mutexes alone, purely for 
performance reasons. We'll have to excuse a couple of `#ifdef 
_LIBCPP_THREAD_API_` conditionals in the library sources to allow the 
external threading API to function.

Does that sound like an OK compromise? @EricWF, @mclow.lists?

Thanks.

/ Asiri

[1] https://mentorembedded.github.io/cxx-abi/abi.html#dso-dtor


http://reviews.llvm.org/D19412



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