[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

2017-05-20 Thread Faisal Vali via Phabricator via cfe-commits
faisalv accepted this revision.
faisalv added a comment.
This revision is now accepted and ready to land.

Modified and committed as: 
http://llvm.org/viewvc/llvm-project?view=revision=303492


https://reviews.llvm.org/D31588



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


[PATCH] D33363: [mips] Support `micromips` attribute

2017-05-20 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:1277-1278
+``__attribute__((nomicromips))`` attributes on MIPS targets. These attributes
+may be attached to a function definition and instructs the backend to locally
+select or turn off microMIPS code generation.
+

> instructs the backend to locally select or turn off microMIPS code generation.

I think we can improve this to:

"instructs the backend to generate or not to generate microMIPS code for that 
function."



Comment at: lib/Sema/SemaDeclAttr.cpp:5949
+  case AttributeList::AT_MicroMips:
+handleSimpleAttributeWithExclusions(S, D, Attr);

This is incorrect. microMIPS can be used to support interrupt handlers as the 
instruction set supports returning from an exception with 'eret'. MIPS16e lacks 
the 'eret' instruction, so it cannot be used with an interrupt handler function.

Aside: it is implementation dependant which ISAmode is used when an interrupt 
occurs on a processor supporting microMIPS and MIPS32R3 from my reading of the 
specification.


Repository:
  rL LLVM

https://reviews.llvm.org/D33363



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


r303492 - Fix PR25627: constant expressions being odr-used in template arguments.

2017-05-20 Thread Faisal Vali via cfe-commits
Author: faisalv
Date: Sat May 20 14:58:04 2017
New Revision: 303492

URL: http://llvm.org/viewvc/llvm-project?rev=303492=rev
Log:
Fix PR25627: constant expressions being odr-used in template arguments.

This patch ensures that clang processes the expression-nodes that are generated 
when disambiguating between types and expressions within template arguments as 
constant-expressions by installing the ConstantEvaluated 
ExpressionEvaluationContext just before attempting the disambiguation - and 
then making sure that Context carries through into ParseConstantExpression (by 
refactoring it out into a function that does not create its own 
EvaluationContext: ParseConstantExpressionInExprEvalContext) 

Note, prior to this patch, trunk would correctly disambiguate and identify the 
expression as an expression - and while it would annotate the token with the 
expression - it would fail to complete the odr-use processing (specifically, 
failing to trigger Sema::UpdateMarkingForLValueToRValue as is done for all 
Constant Expressions, which would remove it from being considered odr-used).  
By installing the ConstantExpression Evaluation Context prior to 
disambiguation, and making sure it carries though, we ensure correct processing 
of the expression-node.

For e.g:
  template struct X { };
  void f() {
const int N = 10;
X x; // should be OK.
[] { return X{}; }; // Should be OK - no capture - but clang errors!
  }

See a related bug: https://bugs.llvm.org//show_bug.cgi?id=25627

In summary (and reiteration), the fix is as follows:

- Remove the EnteredConstantEvaluatedContext action from 
ParseTemplateArgumentList (relying on ParseTemplateArgument getting it right)
- Add the EnteredConstantEvaluatedContext action just prior to undergoing 
the disambiguating parse, and if the parse succeeds for an expression, carry 
the context though into a refactored version of ParseConstantExpression that 
does not create its own ExpressionEvaluationContext.

See https://reviews.llvm.org/D31588 for additional context regarding some of 
the more fragile and complicated approaches attempted, and Richard's feedback 
that eventually shaped the simpler and more robust rendition that is being 
committed.

Thanks Richard!

Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Parse/ParseTemplate.cpp
cfe/trunk/test/SemaCXX/lambda-expressions.cpp
cfe/trunk/test/SemaCXX/local-classes.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=303492=303491=303492=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Sat May 20 14:58:04 2017
@@ -1460,6 +1460,7 @@ public:
   };
 
   ExprResult ParseExpression(TypeCastState isTypeCast = NotTypeCast);
+  ExprResult ParseConstantExpressionInExprEvalContext(TypeCastState 
isTypeCast);
   ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast);
   ExprResult ParseConstraintExpression();
   // Expr that doesn't include commas.

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=303492=303491=303492=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Sat May 20 14:58:04 2017
@@ -192,6 +192,16 @@ Parser::ParseAssignmentExprWithObjCMessa
   return ParseRHSOfBinaryExpression(R, prec::Assignment);
 }
 
+ExprResult
+Parser::ParseConstantExpressionInExprEvalContext(TypeCastState isTypeCast) {
+  assert(Actions.ExprEvalContexts.back().Context ==
+ Sema::ExpressionEvaluationContext::ConstantEvaluated &&
+ "Call this function only if your ExpressionEvaluationContext is "
+ "already ConstantEvaluated");
+  ExprResult LHS(ParseCastExpression(false, false, isTypeCast));
+  ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
+  return Actions.ActOnConstantExpression(Res);
+}
 
 ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast) {
   // C++03 [basic.def.odr]p2:
@@ -200,10 +210,7 @@ ExprResult Parser::ParseConstantExpressi
   // C++98 and C++11 have no such rule, but this is only a defect in C++98.
   EnterExpressionEvaluationContext ConstantEvaluated(
   Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
-
-  ExprResult LHS(ParseCastExpression(false, false, isTypeCast));
-  ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
-  return Actions.ActOnConstantExpression(Res);
+  return ParseConstantExpressionInExprEvalContext(isTypeCast);
 }
 
 /// \brief Parse a constraint-expression.

Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL: 

[PATCH] D31608: [coroutines] Add emission of initial and final suspends

2017-05-20 Thread Gor Nishanov via Phabricator via cfe-commits
GorNishanov added a comment.

@EricWF: Can you take a quick look? Just a sanity check. The change is rather 
trivial.


https://reviews.llvm.org/D31608



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


[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.

2017-05-20 Thread Malhar Thakkar via Phabricator via cfe-commits
malhar1995 added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:143-146
+if(lstate->isSchrodingerUntouched())
+  state = state->remove(lockR);
+else if(lstate->isSchrodingerUnlocked())
+  state = state->set(lockR, LockState::getUnlocked());

NoQ wrote:
> I think we can be certain that the lock is in one of these states, and assert 
> that.
We can be certain that the lock state will be either of the two only if I add 
the following statement before returning from this function.
```
state = state->remove(lockR);
```
If I don't add the above statement, a return value symbol for the region 
specified by lockR will still be in DestroyRetVal and it may have an actual 
lock state (locked, unlocked or destroyed).


Repository:
  rL LLVM

https://reviews.llvm.org/D32449



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


[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-05-20 Thread Stanisław Barzowski via Phabricator via cfe-commits
sbarzowski marked 3 inline comments as done.
sbarzowski added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:25
+  Finder->addMatcher(
+  cxxThrowExpr(stmt(hasAncestor(functionDecl(isNoThrow()).bind("func"
+  .bind("throw"),

Prazek wrote:
> aaron.ballman wrote:
> > Prazek wrote:
> > > you can use forFunction instead of hasAncestor(functionDecl(
> > > cxxThrowExpr(stmt(forFunction(isNoThrow() or 
> > > cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow(
> > > you can even try to remove stmt.
> > > 
> > > 
> > > Also add test case
> > > 
> > >   void getFunction() noexcept {
> > >struct X { 
> > > static void inner()
> > > {
> > > throw 42;
> > > }
> > >}; 
> > >   }
> > > 
> > Hmm, I wonder if it's trivial enough to filter out throw statements that 
> > are inside a try block scope (even without checking the expression and 
> > catch block types)?
> unless(hasAncestor(cxxTryStmt())) should do the work for almost all cases. 
> Maybe even something like this to catch only valid try blocks
> 
>  cxxThrowExpr(stmt(forFunction(functionDecl(isNoThrow()).bind("func"))),
> unless(hasAncestor(cxxTryStmt().bind("try"), 
> forFunction(hasBody(hasDescendant(equalsBoundNode("try")
> 
> + it should check that the throw is not in CXXCatchStmt (unless it is in 
> another try block).
> 
Now even the caught type is checked.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

aaron.ballman wrote:
> sbarzowski wrote:
> > sbarzowski wrote:
> > > vmiklos wrote:
> > > > Prazek wrote:
> > > > > aaron.ballman wrote:
> > > > > > What is the false positive rate with this check over a large 
> > > > > > codebase that uses exceptions?
> > > > > Do you know any good project to check it?
> > > > LibreOffice might be a candidate, see 
> > > >  for details 
> > > > on how to generate a compile database for it (since it does not use 
> > > > cmake), then you can start testing.
> > > Thanks. However it's not just about using exception. To be meaningful I 
> > > need a project that use noexcept in more than a few places.
> > > 
> > > Here are some projects that I found:
> > > https://github.com/facebook/hhvm/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/facebook/folly/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/Tencent/mars/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/SFTtech/openage/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/facebook/watchman/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/facebook/proxygen/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/philsquared/Catch/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93=noexcept
> > > https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93=noexcept
> > > 
> > > I've tried HHVM so far, and ran into some problems compiling it. Anyway 
> > > I'm working on it.
> > Ok, I was able to run it on most of the HHVM  codebase + deps. There were 
> > some issues that looked like some autogenerated pieces missing, so it may 
> > have been not 100% covered.
> > 
> > The results:
> > 3 occurences in total
> > 1) rethrow in destructor (http://pastebin.com/JRNMZGev)
> > 2) false positive "throw and catch in the same function" 
> > (http://pastebin.com/14y1AJgM)
> > 3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)
> > 
> > That's not too many, but this is a kind of check that I guess would be most 
> > useful earlier in the development - ideally before the initial code review.
> > 
> > I'm not sure if we should count (3) as false positive. We could potentially 
> > eliminate it, by checking if the expression in noexcept is empty or true 
> > literal.
> > 
> > (2) is def. a false positive. The potential handling of this case was 
> > already proposed, but I'm not sure if it's worth it.  The code in example 
> > (2) is ugly and extracting these throwing parts to separate functions looks 
> > like a good way to start refactoring. 
> > 
> > What do you think?
> > 
> The fact that there's a false positive in the first large project you checked 
> suggests that the false positive is likely worth it to fix. The code may be 
> ugly, but it's not uncommon -- some coding guidelines explicitly disallow use 
> of gotos, and this is a reasonable(ish) workaround for that.
> 
> As for #3, I would consider that to be a false-positive as well. A computed 
> noexcept clause is going to be very common, especially in generic code. I 
> think 

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-05-20 Thread Stanisław Barzowski via Phabricator via cfe-commits
sbarzowski updated this revision to Diff 99670.
sbarzowski marked 5 inline comments as done.
sbarzowski added a comment.

Cosmetic


https://reviews.llvm.org/D19201

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowWithNoexceptCheck.cpp
  clang-tidy/misc/ThrowWithNoexceptCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-with-noexcept.rst
  test/clang-tidy/misc-throw-with-noexcept.cpp

Index: test/clang-tidy/misc-throw-with-noexcept.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-with-noexcept.cpp
@@ -0,0 +1,201 @@
+// RUN: %check_clang_tidy %s misc-throw-with-noexcept %t
+
+void f_throw_with_ne() noexcept(true) {
+  {
+throw 5;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-6]]:24: note: in a function declared nothrow here
+// CHECK-FIXES: void f_throw_with_ne() {
+
+void f_noexcept_false() noexcept(false) {
+  throw 5;
+}
+
+void f_decl_only() noexcept;
+
+
+void throw_and_catch() noexcept(true) {
+  try {
+throw 5;
+  } catch (...) {
+  }
+}
+
+struct A{};
+struct B{};
+
+void throw_and_catch_something_else() noexcept(true) {
+  try {
+throw A();
+  } catch (B b) {
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-7]]:39: note: in a function declared nothrow here
+// CHECK-FIXES: void throw_and_catch_something_else() {
+
+
+void throw_and_catch_the_same_thing() noexcept {
+  try {
+throw A();
+  } catch (A a) {
+  }
+}
+
+
+void throw_and_catch_int() noexcept {
+  try {
+throw 42;
+  } catch (int a) {
+  }
+}
+
+
+void throw_and_catch_() noexcept {
+  try {
+throw 42;
+  } catch (int a) {
+  }
+}
+
+
+void throw_and_catch_pointer() noexcept {
+  try {
+throw A();
+  } catch (A *a) {
+  }
+}
+
+class Base{};
+class Derived: public Base {};
+
+void throw_and_catch_base_class() noexcept {
+  try {
+throw Derived();
+  } catch (Base ) {
+  }
+}
+
+void throw_and_catch_nested() noexcept {
+  try {
+try {
+throw Derived();
+} catch (int x) {
+}
+  } catch (Base ) {
+  }
+}
+
+void throw_and_catch_derived_class() noexcept {
+  try {
+throw Base();
+  } catch (Derived ) {
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-7]]:38: note: in a function declared nothrow here
+// CHECK-FIXES: void throw_and_catch_derived_class() {
+
+
+class Class {
+  void InClass() const noexcept(true) {
+throw 42;
+  }
+};
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-5]]:24: note: in a function declared nothrow here
+// CHECK-FIXES: void InClass() const {
+
+
+void forward_declared() noexcept;
+
+void forward_declared() noexcept {
+	throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: in a function declared nothrow here
+// CHECK-MESSAGES: :[[@LINE-7]]:25: note: in a function declared nothrow here
+// CHECK-FIXES: void forward_declared() ;
+// CHECK-FIXES: void forward_declared() {
+
+void getFunction() noexcept {
+  struct X {
+static void inner()
+{
+throw 42;
+}
+  };
+}
+
+void dynamic_exception_spec() throw() {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: in a function declared nothrow here
+// CHECK-FIXES: void dynamic_exception_spec() {
+
+#define NOEXCEPT noexcept
+
+void with_macro() NOEXCEPT {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: in a function declared nothrow here
+// CHECK-FIXES: void with_macro() {
+
+template int template_function() noexcept {
+  throw 42;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'throw' expression in a function declared with a non-throwing exception specification [misc-throw-with-noexcept]
+// CHECK-MESSAGES: :[[@LINE-4]]:46: note: in a function declared nothrow here
+// CHECK-FIXES: template int template_function() {
+
+template class template_class {
+  int throw_in_member() noexcept {
+throw 42;
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'throw' expression 

[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

2017-05-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1311
+  EmptyDecl *ToD = EmptyDecl::Create(Importer.getToContext(), DC, Loc);
+  ToD->setLexicalDeclContext(LexicalDC);
+  LexicalDC->addDeclInternal(ToD);

xazax.hun wrote:
> Don't we need an Importer.Imported call here? 
It's done a level upper in `ASTImporter::ImportDecl()` but I think it's worth 
it to add an explicit call.



Comment at: lib/AST/ASTImporter.cpp:1464
+
+  NamespaceDecl *TargetDecl = cast(
+Importer.Import(D->getNamespace()));

szepet wrote:
> Since the Import can result nullptr (which is checked 2 lines below) this 
> should be a cast_or_null as I see.
Nice spot, thank you!


https://reviews.llvm.org/D32751



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


[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

2017-05-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin updated this revision to Diff 99669.
a.sidorin added a comment.

Removed accidentally duplicated comment.


https://reviews.llvm.org/D32751

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/namespace/Inputs/namespace1.cpp
  test/ASTMerge/namespace/Inputs/namespace2.cpp
  test/ASTMerge/namespace/test.cpp

Index: test/ASTMerge/namespace/test.cpp
===
--- test/ASTMerge/namespace/test.cpp
+++ test/ASTMerge/namespace/test.cpp
@@ -1,6 +1,17 @@
-// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/namespace1.cpp
-// RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/namespace2.cpp
-// RUN: not %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/namespace1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/namespace2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(TestAliasName::z == 4);
+static_assert(ContainsInline::z == 10);
+
+void testImport() {
+  typedef TestUnresolvedTypenameAndValueDecls::Derived Imported;
+  Imported a; // Successfull instantiation
+  static_assert(sizeof(Imported::foo) == sizeof(int));
+  static_assert(sizeof(TestUnresolvedTypenameAndValueDecls::Derived::NewUnresolvedUsingType) == sizeof(double));
+}
+
 
 // CHECK: namespace2.cpp:16:17: error: external variable 'z' declared with incompatible types in different translation units ('double' vs. 'float')
 // CHECK: namespace1.cpp:16:16: note: declared here with type 'float'
Index: test/ASTMerge/namespace/Inputs/namespace2.cpp
===
--- test/ASTMerge/namespace/Inputs/namespace2.cpp
+++ test/ASTMerge/namespace/Inputs/namespace2.cpp
@@ -15,3 +15,46 @@
 namespace N3 {
   extern double z;
 }
+
+namespace Enclosing {
+namespace Nested {
+  const int z = 4;
+}
+}
+
+namespace ContainsInline {
+  inline namespace Inline {
+const int z = 10;
+  }
+}
+
+namespace TestAliasName = Enclosing::Nested;
+// NOTE: There is no warning on this alias.
+namespace AliasWithSameName = Enclosing::Nested;
+
+namespace TestUsingDecls {
+
+namespace A {
+void foo();
+}
+namespace B {
+using A::foo; // <- a UsingDecl creating a UsingShadow
+}
+
+}// end namespace TestUsingDecls
+
+namespace TestUnresolvedTypenameAndValueDecls {
+
+template  class Base;
+template  class Derived : public Base {
+public:
+  using typename Base::foo;
+  using Base::bar;
+  typedef typename Derived::foo NewUnresolvedUsingType;
+};
+
+} // end namespace TestUnresolvedTypenameAndValueDecls
+
+namespace TestUsingNamespace {
+  using namespace Enclosing;
+}
Index: test/ASTMerge/namespace/Inputs/namespace1.cpp
===
--- test/ASTMerge/namespace/Inputs/namespace1.cpp
+++ test/ASTMerge/namespace/Inputs/namespace1.cpp
@@ -15,3 +15,13 @@
 namespace N3 {
   extern float z;
 }
+
+namespace AliasWithSameName = N3;
+
+namespace TestUnresolvedTypenameAndValueDecls {
+template  class Base {
+public:
+  typedef T foo;
+  void bar();
+};
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -58,7 +58,7 @@
 QualType VisitExtVectorType(const ExtVectorType *T);
 QualType VisitFunctionNoProtoType(const FunctionNoProtoType *T);
 QualType VisitFunctionProtoType(const FunctionProtoType *T);
-// FIXME: UnresolvedUsingType
+QualType VisitUnresolvedUsingType(const UnresolvedUsingType *T);
 QualType VisitParenType(const ParenType *T);
 QualType VisitTypedefType(const TypedefType *T);
 QualType VisitTypeOfExprType(const TypeOfExprType *T);
@@ -128,8 +128,8 @@
 TemplateParameterList *ImportTemplateParameterList(
  TemplateParameterList *Params);
 TemplateArgument ImportTemplateArgument(const TemplateArgument );
-TemplateArgumentLoc ImportTemplateArgumentLoc(
-const TemplateArgumentLoc , bool );
+Optional ImportTemplateArgumentLoc(
+const TemplateArgumentLoc );
 bool ImportTemplateArguments(const TemplateArgument *FromArgs,
  unsigned NumFromArgs,
SmallVectorImpl );
@@ -142,10 +142,12 @@
 bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
 bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
 Decl *VisitDecl(Decl *D);
+Decl *VisitEmptyDecl(EmptyDecl *D);
 Decl *VisitAccessSpecDecl(AccessSpecDecl *D);
 Decl *VisitStaticAssertDecl(StaticAssertDecl *D);
 Decl *VisitTranslationUnitDecl(TranslationUnitDecl *D);
 Decl *VisitNamespaceDecl(NamespaceDecl *D);
+Decl *VisitNamespaceAliasDecl(NamespaceAliasDecl *D);
 Decl *VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias);
 Decl 

[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

2017-05-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin updated this revision to Diff 99668.
a.sidorin added a comment.

Replaced cast<> with cast_or_null<>.


https://reviews.llvm.org/D32751

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/namespace/Inputs/namespace1.cpp
  test/ASTMerge/namespace/Inputs/namespace2.cpp
  test/ASTMerge/namespace/test.cpp

Index: test/ASTMerge/namespace/test.cpp
===
--- test/ASTMerge/namespace/test.cpp
+++ test/ASTMerge/namespace/test.cpp
@@ -1,6 +1,17 @@
-// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/namespace1.cpp
-// RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/namespace2.cpp
-// RUN: not %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/namespace1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/namespace2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(TestAliasName::z == 4);
+static_assert(ContainsInline::z == 10);
+
+void testImport() {
+  typedef TestUnresolvedTypenameAndValueDecls::Derived Imported;
+  Imported a; // Successfull instantiation
+  static_assert(sizeof(Imported::foo) == sizeof(int));
+  static_assert(sizeof(TestUnresolvedTypenameAndValueDecls::Derived::NewUnresolvedUsingType) == sizeof(double));
+}
+
 
 // CHECK: namespace2.cpp:16:17: error: external variable 'z' declared with incompatible types in different translation units ('double' vs. 'float')
 // CHECK: namespace1.cpp:16:16: note: declared here with type 'float'
Index: test/ASTMerge/namespace/Inputs/namespace2.cpp
===
--- test/ASTMerge/namespace/Inputs/namespace2.cpp
+++ test/ASTMerge/namespace/Inputs/namespace2.cpp
@@ -15,3 +15,46 @@
 namespace N3 {
   extern double z;
 }
+
+namespace Enclosing {
+namespace Nested {
+  const int z = 4;
+}
+}
+
+namespace ContainsInline {
+  inline namespace Inline {
+const int z = 10;
+  }
+}
+
+namespace TestAliasName = Enclosing::Nested;
+// NOTE: There is no warning on this alias.
+namespace AliasWithSameName = Enclosing::Nested;
+
+namespace TestUsingDecls {
+
+namespace A {
+void foo();
+}
+namespace B {
+using A::foo; // <- a UsingDecl creating a UsingShadow
+}
+
+}// end namespace TestUsingDecls
+
+namespace TestUnresolvedTypenameAndValueDecls {
+
+template  class Base;
+template  class Derived : public Base {
+public:
+  using typename Base::foo;
+  using Base::bar;
+  typedef typename Derived::foo NewUnresolvedUsingType;
+};
+
+} // end namespace TestUnresolvedTypenameAndValueDecls
+
+namespace TestUsingNamespace {
+  using namespace Enclosing;
+}
Index: test/ASTMerge/namespace/Inputs/namespace1.cpp
===
--- test/ASTMerge/namespace/Inputs/namespace1.cpp
+++ test/ASTMerge/namespace/Inputs/namespace1.cpp
@@ -15,3 +15,13 @@
 namespace N3 {
   extern float z;
 }
+
+namespace AliasWithSameName = N3;
+
+namespace TestUnresolvedTypenameAndValueDecls {
+template  class Base {
+public:
+  typedef T foo;
+  void bar();
+};
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -58,7 +58,7 @@
 QualType VisitExtVectorType(const ExtVectorType *T);
 QualType VisitFunctionNoProtoType(const FunctionNoProtoType *T);
 QualType VisitFunctionProtoType(const FunctionProtoType *T);
-// FIXME: UnresolvedUsingType
+QualType VisitUnresolvedUsingType(const UnresolvedUsingType *T);
 QualType VisitParenType(const ParenType *T);
 QualType VisitTypedefType(const TypedefType *T);
 QualType VisitTypeOfExprType(const TypeOfExprType *T);
@@ -128,8 +128,8 @@
 TemplateParameterList *ImportTemplateParameterList(
  TemplateParameterList *Params);
 TemplateArgument ImportTemplateArgument(const TemplateArgument );
-TemplateArgumentLoc ImportTemplateArgumentLoc(
-const TemplateArgumentLoc , bool );
+Optional ImportTemplateArgumentLoc(
+const TemplateArgumentLoc );
 bool ImportTemplateArguments(const TemplateArgument *FromArgs,
  unsigned NumFromArgs,
SmallVectorImpl );
@@ -142,10 +142,12 @@
 bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
 bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
 Decl *VisitDecl(Decl *D);
+Decl *VisitEmptyDecl(EmptyDecl *D);
 Decl *VisitAccessSpecDecl(AccessSpecDecl *D);
 Decl *VisitStaticAssertDecl(StaticAssertDecl *D);
 Decl *VisitTranslationUnitDecl(TranslationUnitDecl *D);
 Decl *VisitNamespaceDecl(NamespaceDecl *D);
+Decl *VisitNamespaceAliasDecl(NamespaceAliasDecl *D);
 Decl *VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias);
 Decl 

[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.

2017-05-20 Thread Malhar Thakkar via Phabricator via cfe-commits
malhar1995 marked 2 inline comments as done.
malhar1995 added a comment.

In https://reviews.llvm.org/D32449#760141, @NoQ wrote:

> Thanks! Your code looks very clear now, and it seems correct to me.
>
> One last thing we definitely should do here would be add regression tests for 
> the new functionality. I guess you already have your tests, otherwise you 
> wouldn't know if your code works, so you'd just need to append them to the 
> patch, somewhere at the bottom of `test/Analysis/pthreadlock.c`, and make 
> sure that `make -j4 check-clang-analysis` passes. Ideally, we should cover as 
> many branches as possible.
>
> A few ideas of what to test (you might have thought about most of them 
> already, and i expect them to actually work by just looking at what your code 
> accomplishes):
>
> - What we can/cannot do with the mutex in the failed-to-be-destroyed state, 
> depending on the state of the mutex before destruction was attempted.
> - What we can/cannot do with the mutex in each of the "Schrodinger" states - 
> in particular, do we display the double-destroy warning in such cases?
> - How return-symbol death races against resolving success of the destroy 
> operation: what if the programmer first tries to destroy mutex, then uses the 
> mutex, then checks the return value?
> - Are you sure we cannot `assert(lstate)` on line 137? - a test could be 
> added that would cause such assertion to fail if someone tries to impose it.
>
>   Apart from that, i guess it'd be good to use more informative variable 
> names in a few places (see inline comments), and fix the formatting, i.e. 
> spaces and line breaks (should be easy with `clang-format`). Also you 
> shouldn't add the `.DS_Store` files :) And then we'd accept and commit this 
> patch.


Dear Dr. Artem,

Thank you so much for such a detailed review. I'll work on addressing these 
comments ASAP and reach out to you in case I have any queries.

Regards,
Malhar Thakkar


Repository:
  rL LLVM

https://reviews.llvm.org/D32449



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


[PATCH] D32936: [clang] Add support for Ananas platform

2017-05-20 Thread Ed Schouten via Phabricator via cfe-commits
ed accepted this revision.
ed added a comment.
This revision is now accepted and ready to land.

These changes look all right to me, except that I'd like it if someone else 
could take a look at `Ananas.cpp` as well. Is anyone interested in taking a 
look?


https://reviews.llvm.org/D32936



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


[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-05-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

As an FYI, there is a related check being implemented in clang currently; we 
probably should not duplicate this effort. See https://reviews.llvm.org/D3.


https://reviews.llvm.org/D19201



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


[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

2017-05-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

As an FYI, there is a related check currently under development in clang-tidy; 
we probably should not duplicate this functionality in both places. See 
https://reviews.llvm.org/D19201 for the other review.


https://reviews.llvm.org/D3



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


[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.

2017-05-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Thanks! Your code looks very clear now, and it seems correct to me.

One last thing we definitely should do here would be add regression tests for 
the new functionality. I guess you already have your tests, otherwise you 
wouldn't know if your code works, so you'd just need to append them to the 
patch, somewhere at the bottom of `test/Analysis/pthreadlock.c`, and make sure 
that `make -j4 check-clang-analysis` passes. Ideally, we should cover as many 
branches as possible.

A few ideas of what to test (you might have thought about most of them already, 
and i expect them to actually work by just looking at what your code 
accomplishes):

- What we can/cannot do with the mutex in the failed-to-be-destroyed state, 
depending on the state of the mutex before destruction was attempted.
- What we can/cannot do with the mutex in each of the "Schrodinger" states - in 
particular, do we display the double-destroy warning in such cases?
- How return-symbol death races against resolving success of the destroy 
operation: what if the programmer first tries to destroy mutex, then uses the 
mutex, then checks the return value?
- Are you sure we cannot `assert(lstate)` on line 137? - a test could be added 
that would cause such assertion to fail if someone tries to impose it.

Apart from that, i guess it'd be good to use more informative variable names in 
a few places (see inline comments), and fix the formatting, i.e. spaces and 
line breaks (should be easy with `clang-format`). Also you shouldn't add the 
`.DS_Store` files :) And then we'd accept and commit this patch.




Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:28
 struct LockState {
-  enum Kind { Destroyed, Locked, Unlocked } K;
+  enum Kind { Destroyed, Locked, Unlocked, SchrodingerUntouched, 
SchrodingerUnlocked } K;
 

I still think these names, no matter if a good metaphor or not and no matter 
how much i enjoyed them, should be toned down :) Suggesting 
`UntouchedAndPossiblyDestroyed` and `UnlockedAndPossiblyDestroyed`.



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:77
   void reportUseDestroyedBug(CheckerContext , const CallExpr *CE) const;
+  ProgramStateRef setAppropriateLockState(ProgramStateRef state, const 
MemRegion* lockR, const SymbolRef* sym, bool fromCheckDeadSymbols) const;
 };

I suggest renaming to something like "`resolvePossiblyDestroyedMutex()`".

Also, i'm for passing the symbol by value (with `*` dereference at most call 
sites) because it's less surprising/confusing to the reader.

I also suggest a comment explaining what the function does. Eg., "When a lock 
is destroyed, in some semantics we are not sure if the destroy call has 
succeeded or failed, and the lock enters one of the 'possibly destroyed' state. 
There is a short time frame for the programmer to check the return value to see 
if the lock was successfully destroyed. Before we model the next operation over 
that lock, we call this function to see if the return value was checked by now 
and set the lock state - either to destroyed state or back to its previous 
state."



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:81-85
 // GDM Entry for tracking lock state.
 REGISTER_LIST_WITH_PROGRAMSTATE(LockSet, const MemRegion *)
 
 REGISTER_MAP_WITH_PROGRAMSTATE(LockMap, const MemRegion *, LockState)
+REGISTER_MAP_WITH_PROGRAMSTATE(DestroyRetVal, const MemRegion *, SymbolRef)

Because there's only one comment per three traits, it'd be great to clean this 
up a bit together with commenting up your new trait:

```
// A stack of locks for tracking lock-unlock order.
REGISTER_LIST_WITH_PROGRAMSTATE(LockSet, const MemRegion *)

// An entry for tracking lock states.
REGISTER_MAP_WITH_PROGRAMSTATE(LockMap, const MemRegion *, LockState)

// Return values for unresolved destroy calls.
REGISTER_MAP_WITH_PROGRAMSTATE(DestroyRetVal, const MemRegion *, SymbolRef)
```



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:143-146
+if(lstate->isSchrodingerUntouched())
+  state = state->remove(lockR);
+else if(lstate->isSchrodingerUnlocked())
+  state = state->set(lockR, LockState::getUnlocked());

I think we can be certain that the lock is in one of these states, and assert 
that.



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:149-151
+if(lstate->isSchrodingerUntouched() || lstate->isSchrodingerUnlocked()){
+  state = state->set(lockR, LockState::getDestroyed());
+}

Assert the lock state here as well?



Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:302-308
+  SVal X = State->getSVal(CE, C.getLocationContext());
+  if (X.isUnknownOrUndef()){
+return;
+  }
+
+  DefinedSVal retVal = X.castAs();
+  SymbolRef sym = retVal.getAsSymbol();

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-05-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments.



Comment at: docs/ReleaseNotes.rst:58
 
+- New `misc-throw-with-noexcept
+  
`_ 
check

I think this should be in alphabetical order.


https://reviews.llvm.org/D19201



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


[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

2017-05-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:76
+
+  for (const auto Parent : Context->getParents(node)) {
+if (isCaughtInFunction(Context, Throw, Function, Parent))

unnecessary braces



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:113-115
+  if (isCaughtInFunction(Result.Context, Throw, Function, ThrowNode)) {
+return;
+  }

unnecessary braces



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:126-131
+
+  for (const auto  : NoExceptRanges) {
+diag(NoExceptRange.getBegin(),
+ "in a function declared nothrow here", DiagnosticIDs::Note)
+<< FixItHint::CreateRemoval(NoExceptRange);
+  }

same here


https://reviews.llvm.org/D19201



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


[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-20 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a reviewer: mehdi_amini.
Prazek added a comment.

Ping


https://reviews.llvm.org/D31830



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


[PATCH] D32401: [Devirtualization] insert placement new barrier with -O0

2017-05-20 Thread Piotr Padlewski via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303488: [Devirtualization] insert placement new barrier with 
-O0 (authored by Prazek).

Changed prior to commit:
  https://reviews.llvm.org/D32401?vs=96313=99667#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32401

Files:
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp


Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp
@@ -1658,8 +1658,9 @@
 
   // Passing pointer through invariant.group.barrier to avoid propagation of
   // vptrs information which may be included in previous type.
+  // To not break LTO with different optimizations levels, we do it regardless
+  // of optimization level.
   if (CGM.getCodeGenOpts().StrictVTablePointers &&
-  CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   allocator->isReservedGlobalPlacementOperator())
 result = Address(Builder.CreateInvariantGroupBarrier(result.getPointer()),
  result.getAlignment());


Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp
@@ -1658,8 +1658,9 @@
 
   // Passing pointer through invariant.group.barrier to avoid propagation of
   // vptrs information which may be included in previous type.
+  // To not break LTO with different optimizations levels, we do it regardless
+  // of optimization level.
   if (CGM.getCodeGenOpts().StrictVTablePointers &&
-  CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   allocator->isReservedGlobalPlacementOperator())
 result = Address(Builder.CreateInvariantGroupBarrier(result.getPointer()),
  result.getAlignment());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r303488 - [Devirtualization] insert placement new barrier with -O0

2017-05-20 Thread Piotr Padlewski via cfe-commits
Author: prazek
Date: Sat May 20 03:56:18 2017
New Revision: 303488

URL: http://llvm.org/viewvc/llvm-project?rev=303488=rev
Log:
[Devirtualization] insert placement new barrier with -O0

Summary:
To not break LTO with different optimizations levels, we should insert
the barrier regardles of optimization level.

Reviewers: rjmccall, rsmith, mehdi_amini

Reviewed By: mehdi_amini

Subscribers: mehdi_amini, cfe-commits

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

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

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=303488=303487=303488=diff
==
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sat May 20 03:56:18 2017
@@ -1658,8 +1658,9 @@ llvm::Value *CodeGenFunction::EmitCXXNew
 
   // Passing pointer through invariant.group.barrier to avoid propagation of
   // vptrs information which may be included in previous type.
+  // To not break LTO with different optimizations levels, we do it regardless
+  // of optimization level.
   if (CGM.getCodeGenOpts().StrictVTablePointers &&
-  CGM.getCodeGenOpts().OptimizationLevel > 0 &&
   allocator->isReservedGlobalPlacementOperator())
 result = Address(Builder.CreateInvariantGroupBarrier(result.getPointer()),
  result.getAlignment());


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