[PATCH] D50953: [clang-tidy] Handle sugared reference types in ExprMutationAnalyzer

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang added a comment.

In https://reviews.llvm.org/D50953#1229287, @JonasToth wrote:

> What happens to pointers in a typedef (in the sense of `*` instead of `&`)?


I checked around and I believe reference type is the only type we're explicitly 
matching right now. We'll need to handle carefully when handling pointer types 
in the future.




Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:51
 const auto nonConstReferenceType = [] {
-  return referenceType(pointee(unless(isConstQualified(;
+  return hasUnqualifiedDesugaredType(
+  referenceType(pointee(unless(isConstQualified();

JonasToth wrote:
> Not directly related, but this matcher matches universal references, even 
> though they might result in a value. (one of the bugs lebedevri reported).
Noted, I'll take a look at the bug.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50953



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

[PATCH] D50953: [clang-tidy] Handle sugared reference types in ExprMutationAnalyzer

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang updated this revision to Diff 164806.
shuaiwang marked 2 inline comments as done.
shuaiwang added a comment.

more test cases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50953

Files:
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -191,6 +191,24 @@
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
 
+  AST = tooling::buildASTFromCode("typedef int& IntRef;"
+  "void g(IntRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST =
+  tooling::buildASTFromCode("template  using TRef = T&;"
+"void g(TRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  struct identity { using type = T; };"
+  "template  void g(typename identity::type);"
+  "void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
   AST = tooling::buildASTFromCode(
   "void f() { struct A {}; A operator+(A&, int); A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -214,6 +232,25 @@
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
+  AST = tooling::buildASTFromCode("typedef const int& CIntRef;"
+  "void g(CIntRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "template  using CTRef = const T&;"
+  "void g(CTRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "template  struct identity { using type = T; };"
+  "template "
+  "void g(typename identity::type);"
+  "void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
   AST = tooling::buildASTFromCode(
   "void f() { struct A {}; A operator+(const A&, int); A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -440,22 +477,44 @@
 }
 
 TEST(ExprMutationAnalyzerTest, FollowRefModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
   "void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
   "int& r3 = r2; r3 = 10; }");
-  const auto Results =
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
   ElementsAre("r0", "r1", "r2", "r3", "r3 = 10"));
+
+  AST = tooling::buildASTFromCode(
+  "typedef int& IntRefX;"
+  "using IntRefY = int&;"
+  "void f() { int x; IntRefX r0 = x; IntRefY r1 = r0;"
+  "decltype((x)) r2 = r1; r2 = 10; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("r0", "r1", "r2", "r2 = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, FollowRefNotModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
   "void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
   "int& r3 = r2; int& r4 = r3; int& r5 = r4;}");
-  const auto Results =
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "void f() { int x; int& r0 = x; const int& r1 = r0;}");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "typedef const int& CIntRefX;"
+  "using CIntRefY = const int&;"
+  "void f() { int x; int& r0 = x; CIntRefX r1 = r0;"
+  "CIntRefY r2 = r1; decltype((r1)) r3 = r2;}");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest,

r341902 - [Modules] Add imported modules to the output of -module-file-info

2018-09-10 Thread Bruno Cardoso Lopes via cfe-commits 
Author: bruno
Date: Mon Sep 10 22:17:13 2018
New Revision: 341902

URL: http://llvm.org/viewvc/llvm-project?rev=341902=rev
Log:
[Modules] Add imported modules to the output of -module-file-info

Fix a bug in the deserialization of IMPORTS section and allow for
imported modules to also be printed with -module-file-info.

rdar://problem/43867753

Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/module_file_info.m

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=341902=341901=341902=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Mon Sep 10 22:17:13 2018
@@ -232,7 +232,7 @@ public:
 
   /// If needsImportVisitation returns \c true, this is called for each
   /// AST file imported by this AST file.
-  virtual void visitImport(StringRef Filename) {}
+  virtual void visitImport(StringRef ModuleName, StringRef Filename) {}
 
   /// Indicates that a particular module file extension has been read.
   virtual void readModuleFileExtension(

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=341902=341901=341902=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Mon Sep 10 22:17:13 2018
@@ -601,6 +601,17 @@ namespace {
 
   return true;
 }
+
+/// Returns true if this \c ASTReaderListener wants to receive the
+/// imports of the AST file via \c visitImport, false otherwise.
+bool needsImportVisitation() const override { return true; }
+
+/// If needsImportVisitation returns \c true, this is called for each
+/// AST file imported by this AST file.
+void visitImport(StringRef ModuleName, StringRef Filename) override {
+  Out.indent(2) << "Imports module '" << ModuleName
+<< "': " << Filename.str() << "\n";
+}
 #undef DUMP_BOOLEAN
   };
 }

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=341902=341901=341902=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Sep 10 22:17:13 2018
@@ -4868,11 +4868,11 @@ bool ASTReader::readASTFileControlBlock(
   unsigned Idx = 0, N = Record.size();
   while (Idx < N) {
 // Read information about the AST file.
-Idx += 5; // ImportLoc, Size, ModTime, Signature
-SkipString(Record, Idx); // Module name; FIXME: pass to listener?
+Idx += 1+1+1+1+5; // Kind, ImportLoc, Size, ModTime, Signature
+std::string ModuleName = ReadString(Record, Idx);
 std::string Filename = ReadString(Record, Idx);
 ResolveImportedPath(Filename, ModuleDir);
-Listener.visitImport(Filename);
+Listener.visitImport(ModuleName, Filename);
   }
   break;
 }

Modified: cfe/trunk/test/Modules/module_file_info.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module_file_info.m?rev=341902=341901=341902=diff
==
--- cfe/trunk/test/Modules/module_file_info.m (original)
+++ cfe/trunk/test/Modules/module_file_info.m Mon Sep 10 22:17:13 2018
@@ -16,6 +16,7 @@
 
 // CHECK: Module name: DependsOnModule
 // CHECK: Module map file: {{.*}}DependsOnModule.framework{{[/\\]}}module.map
+// CHECK: Imports module 'Module': {{.*}}Module.pcm
 
 // CHECK: Language options:
 // CHECK:   C99: Yes


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

[PATCH] D51910: [Modules] Add platform feature to requires clause

2018-09-10 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits 
bruno created this revision.
bruno added reviewers: rsmith, v.g.vassilev, aprantl.
Herald added subscribers: dexonsmith, srhines.

Allows module map writers to add build requirements based on platform/os. 
Useful when target features and language dialects aren't enough to 
conditionalize building a module. Among other things, it also makes it more 
convenient for modules with the same name but different set of submodules to be 
described in the same module map.

rdar://problem/43909745


Repository:
  rC Clang

https://reviews.llvm.org/D51910

Files:
  docs/Modules.rst
  lib/Basic/Module.cpp
  test/Modules/target-platform-features.m


Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,33 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build module map file
+// RUN: echo "module RequiresMacOS {" >> %t/Inputs/module.map
+// RUN: echo "  requires macos"   >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "  requires !ios"   >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresMain {" >> %t/Inputs/module.map
+// RUN: echo "  module SubRequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "requires !ios"   >> %t/Inputs/module.map
+// RUN: echo "  }">> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs 
-verify %s -fblocks -fobjc-arc
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs 
%s -fblocks -fobjc-arc 2> %t.notios
+// RUN: FileCheck %s -check-prefix=CHECK-IOS < %t.notios
+
+// CHECK-IOS: module 'RequiresMacOS' requires feature 'macos'
+@import RequiresMacOS;
+// CHECK-IOS: module 'RequiresNotiOS' is incompatible with feature 'ios'
+@import RequiresNotiOS;
+
+// We should never get errors for submodules that don't match
+// CHECK-IOS-NOT: module 'RequiresMain'
+@import RequiresMain;
Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -93,7 +93,8 @@
 .Case("opencl", LangOpts.OpenCL)
 .Case("tls", Target.isTLSSupported())
 .Case("zvector", LangOpts.ZVector)
-.Default(Target.hasFeature(Feature));
+.Default(Target.hasFeature(Feature) ||
+ Target.getPlatformName() == Feature);
   if (!HasFeature)
 HasFeature = std::find(LangOpts.ModuleFeatures.begin(),
LangOpts.ModuleFeatures.end(),
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -466,6 +466,8 @@
 *target feature*
   A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
 
+*platform variant*
+  A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, 
``win32``, ``linux``, etc) is available.
 
 **Example:** The ``std`` module can be extended to also include C++ and C++11 
headers using a *requires-declaration*:
 


Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,33 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build module map file
+// RUN: echo "module RequiresMacOS {" >> %t/Inputs/module.map
+// RUN: echo "  requires macos"   >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "  requires !ios"   >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module RequiresMain {" >> %t/Inputs/module.map
+// RUN: echo "  module SubRequiresNotiOS {" >> %t/Inputs/module.map
+// RUN: echo "requires !ios"   >> %t/Inputs/module.map
+// RUN: echo "  }">> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/Inputs -verify %s -fblocks -fobjc-arc
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -fmodules -fimplicit-module-maps

[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2018-09-10 Thread Tanoy Sinha via Phabricator via cfe-commits 
tks2103 added a comment.

ping @GorNishanov


Repository:
  rC Clang

https://reviews.llvm.org/D51741



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

[clang-tools-extra] r341891 - Revert "Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer""

2018-09-10 Thread Shuai Wang via cfe-commits 
Author: shuaiwang
Date: Mon Sep 10 19:23:35 2018
New Revision: 341891

URL: http://llvm.org/viewvc/llvm-project?rev=341891=rev
Log:
Revert "Revert "[clang-tidy] Handle unresolved expressions in 
ExprMutationAnalyzer""

This is the same as D50619 plus fixes for buildbot failures on windows.
The test failures on windows are caused by -fdelayed-template-parsing
and is fixed by forcing -fno-delayed-template-parsing on test cases that
requires AST for uninstantiated templates.

Modified:
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341891=341890=341891=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Mon Sep 
10 19:23:35 2018
@@ -145,11 +145,16 @@ const Stmt *ExprMutationAnalyzer::findDi
 hasUnaryOperand(equalsNode(Exp)));
 
   // Invoking non-const member function.
+  // A member function is assumed to be non-const when it is unresolved.
   const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
   const auto AsNonConstThis =
   expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), 
on(equalsNode(Exp))),
  cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp);
+ hasArgument(0, equalsNode(Exp))),
+ callExpr(callee(expr(anyOf(
+ 
unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
+ cxxDependentScopeMemberExpr(
+ hasObjectExpression(equalsNode(Exp);
 
   // Taking address of 'Exp'.
   // We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -165,10 +170,16 @@ const Stmt *ExprMutationAnalyzer::findDi
unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
 
   // Used as non-const-ref argument when calling a function.
+  // An argument is assumed to be non-const-ref when the function is 
unresolved.
   const auto NonConstRefParam = forEachArgumentWithParam(
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
-  const auto AsNonConstRefArg =
-  anyOf(callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam));
+  const auto AsNonConstRefArg = anyOf(
+  callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam),
+  callExpr(callee(expr(anyOf(unresolvedLookupExpr(), 
unresolvedMemberExpr(),
+ cxxDependentScopeMemberExpr(),
+ hasType(templateTypeParmType(),
+   hasAnyArgument(equalsNode(Exp))),
+  cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp;
 
   // Captured by a lambda by reference.
   // If we're initializing a capture with 'Exp' directly then we're 
initializing
@@ -195,9 +206,12 @@ const Stmt *ExprMutationAnalyzer::findDi
 
 const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
   // Check whether any member of 'Exp' is mutated.
-  const auto MemberExprs = match(
-  findAll(memberExpr(hasObjectExpression(equalsNode(Exp))).bind("expr")),
-  Stm, Context);
+  const auto MemberExprs =
+  
match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
+   cxxDependentScopeMemberExpr(
+   hasObjectExpression(equalsNode(Exp)
+.bind("expr")),
+Stm, Context);
   return findExprMutation(MemberExprs);
 }
 

Modified: 
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=341891=341890=341891=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
Mon Sep 10 19:23:35 2018
@@ -114,6 +114,29 @@ TEST(ExprMutationAnalyzerTest, NonConstM
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
+TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
+  auto AST = tooling::buildASTFromCodeWithArgs(
+  "struct X { template  void mf(); };"
+  "template  void f() { X x; x.mf(); }",
+  {"-fno-delayed-template-parsing"});
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCodeWithArgs(
+

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

2018-09-10 Thread Josh Gao via Phabricator via cfe-commits 
jmgao added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3016-3017
+def warn_thread_attribute_not_on_capability_member : Warning<
+  "%0 attribute without capability arguments can only be applied in a class "
+  "annotated with 'capability' or 'scoped_lockable' attribute, but %1 isn't">,
+  InGroup, DefaultIgnore;

aaronpuchert wrote:
> Alternative wording: "%0 attribute without capability arguments refers to 
> 'this', but %1 isn't annotated with 'capability' or 'scoped_lockable' 
> attribute".
Maybe something like "implicit 'this' argument for %0 attribute isn't annotated 
with 'capability' or 'scoped_lockable" attribute"?


Repository:
  rC Clang

https://reviews.llvm.org/D51901



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

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

2018-09-10 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added a comment.

The second warning message is a bit long, so if any of you have a better idea 
I'd welcome it.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3016-3017
+def warn_thread_attribute_not_on_capability_member : Warning<
+  "%0 attribute without capability arguments can only be applied in a class "
+  "annotated with 'capability' or 'scoped_lockable' attribute, but %1 isn't">,
+  InGroup, DefaultIgnore;

Alternative wording: "%0 attribute without capability arguments refers to 
'this', but %1 isn't annotated with 'capability' or 'scoped_lockable' 
attribute".


Repository:
  rC Clang

https://reviews.llvm.org/D51901



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a comment.

Sorry, didn't notice this was recently added, from looking at Darwin and 
Android embedded tests, this doesn't really match up with either of them. The 
hosted tests for Android using `libbuiltins` look like this:

  // RUN: %clang -target arm-linux-androideabi -rtlib=compiler-rt -### %s 2>&1 
| FileCheck %s -check-prefix ARM-ANDROID
  // ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins-arm-android.a"

Not sure what to make of this to be honest, I don't think this should be passed 
in as a conventional library since the driver usually picks the correct runtime 
for a given target triple.


https://reviews.llvm.org/D51899



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

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

2018-09-10 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley, jmgao.
Herald added a subscriber: cfe-commits.

When thread safety annotations are used without capability arguments,
they are assumed to apply to `this` instead. So we warn when either
`this` doesn't exist, or the class is not a capability type.

This is based on earlier work by Josh Gao that was committed in r310403,
but reverted in r310698 because it didn't properly work in template
classes. See also https://reviews.llvm.org/D36237.

The solution is not to go via the QualType of `this`, which is then a
template type, hence the attributes are not known because it could be
specialized. Instead we look directly at the class in which we are
contained.

Additionally I grouped two of the warnings together. There are two
issues here: the existence of `this`, which requires us to be a
non-static member function, and the appropriate annotation on the class
we are contained in. So we don't distinguish between not being in a
class and being static, because in both cases we don't have `this`.

Fixes PR38399.


Repository:
  rC Clang

https://reviews.llvm.org/D51901

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-capabilities.c
  test/SemaCXX/warn-thread-safety-parsing.cpp

Index: test/SemaCXX/warn-thread-safety-parsing.cpp
===
--- test/SemaCXX/warn-thread-safety-parsing.cpp
+++ test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -572,11 +572,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void elf_function() EXCLUSIVE_LOCK_FUNCTION();
+void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
 void elf_function_args() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2);
 
-int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION();
+int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
 int elf_testfn(int y) {
   int x EXCLUSIVE_LOCK_FUNCTION() = y; // \
@@ -591,7 +591,8 @@
  private:
   int test_field EXCLUSIVE_LOCK_FUNCTION(); // \
 // expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_LOCK_FUNCTION();
+  void test_method() EXCLUSIVE_LOCK_FUNCTION(); // \
+// expected-warning {{'exclusive_lock_function' attribute without capability arguments can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'ElfFoo' isn't}}
 };
 
 class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -644,11 +645,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void slf_function() SHARED_LOCK_FUNCTION();
+void slf_function() SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
 void slf_function_args() SHARED_LOCK_FUNCTION(mu1, mu2);
 
-int slf_testfn(int y) SHARED_LOCK_FUNCTION();
+int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
 int slf_testfn(int y) {
   int x SHARED_LOCK_FUNCTION() = y; // \
@@ -666,7 +667,8 @@
  private:
   int test_field SHARED_LOCK_FUNCTION(); // \
 // expected-warning {{'shared_lock_function' attribute only applies to functions}}
-  void test_method() SHARED_LOCK_FUNCTION();
+  void test_method() SHARED_LOCK_FUNCTION(); // \
+// expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'SlfFoo' isn't}}
 };
 
 class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -717,14 +719,16 @@
 // takes a mandatory boolean or integer argument specifying the retval
 // plus an optional list of locks (vars/fields)
 
-void etf_function() __attribute__((exclusive_trylock_function));  // \
+void etf_function() __attribute__((exclusive_trylock_function)); // \
   // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}}
 
 void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
 
-void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1);
+void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
-int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1);
+int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
 
 int etf_testfn(int y) {
   int x

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Robert Widmann via Phabricator via cfe-commits 
CodaFi added a comment.

The regression tests are only against the text of the driver's cc1 commands.  
It's not actually testing if we can compile.


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a reviewer: t.p.northover.
kristina added a comment.

Added the main ARM code owner as a reviewer, I'm not entirely sure why you're 
getting this behavior since regression tests should have weeded it out.


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Robert Widmann via Phabricator via cfe-commits 
CodaFi added a comment.

@kristina They're having linker troubles because this driver is hardcoding an 
incorrect linker flag for libbuiltin on the arm-baremetal triple.  
https://reviews.llvm.org/D33259 introduced this incorrect behavior by smashing 
together the incorrect `-lclang_rt.builtins-armv6m` flag with the proper 
resource-directory-relative absolute path 
`/lib/libclang_rt.builtins-armv6m.a`.  The driver should be 
forming an exact path to this library relative to the resource directory.  If 
users wish to override this behavior, they need to pass `-nostdlib` or ` 
-nodefaultlibs`


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a comment.

The library shouldn't be an object file (it could be if you changed some stuff 
around but typically isn't), `libbuiltins` is a combination of multiple 
translation units, if you look at the source. I'm not sure how it's turning 
into an object file for you and even if it was you can pass an archive to the 
driver on the commandline and it would treat it like a group of objects. What 
is the generated output you're getting when you build it for bare metal?


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Mehdi AMINI via Phabricator via cfe-commits 
mehdi_amini added a comment.

@kristina both bfd and lld are adding the  `.a ` extension when you pass a lib 
through -l. Similarly to -llibfoo won't work, -lfoo.a will lead the linker to 
look for libfoo.a.a


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread George Morgan via Phabricator via cfe-commits 
georgemorgan added a comment.

@kristina, the library isn't being passed as an object file to the linker but 
rather as a library name. Hence, the `.a` suffix is not correct.


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a comment.

Please submit your diffs with context in the future (`-U9`). Also this 
seems like it would break a lot, `.a` is just a generic extension for an object 
archive file, I don't think the extension is the issue unless you're using some 
sort of an exotic linker.


https://reviews.llvm.org/D51899



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread George Morgan via Phabricator via cfe-commits 
georgemorgan updated this revision to Diff 164776.
georgemorgan added a comment.

Update diff to provide context.


https://reviews.llvm.org/D51899

Files:
  lib/Driver/ToolChains/BareMetal.cpp
  test/Driver/baremetal.cpp


Index: test/Driver/baremetal.cpp
===
--- test/Driver/baremetal.cpp
+++ test/Driver/baremetal.cpp
@@ -13,7 +13,7 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" 
"-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -35,7 +35,7 @@
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -48,7 +48,7 @@
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -61,7 +61,7 @@
 // CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBSTDCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
-// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-LIBSTDCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: lib/Driver/ToolChains/BareMetal.cpp
===
--- lib/Driver/ToolChains/BareMetal.cpp
+++ lib/Driver/ToolChains/BareMetal.cpp
@@ -157,7 +157,7 @@
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
   CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName() + ".a"));
+   getTriple().getArchName()));
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,


Index: test/Driver/baremetal.cpp
===
--- test/Driver/baremetal.cpp
+++ test/Driver/baremetal.cpp
@@ -13,7 +13,7 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -35,7 +35,7 @@
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -48,7 +48,7 @@
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread Robert Widmann via Phabricator via cfe-commits 
CodaFi added a comment.

Two things:

- Please re-upload this path with with context (`git diff -U9` or `arc 
patch`)
- This is one answer, but the real problem is that this path is halfway between 
an absolute path and a link-relative path.  We should be searching for these 
libraries relative to the resource directory and at absolute paths like the 
other platform's drivers do.  To that end, keep the ".a", but change the `l` to 
`lib` and see `MachO::AddLinkRuntimeLib` for how to add on the resource dir.


Repository:
  rC Clang

https://reviews.llvm.org/D51899



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

[PATCH] D51898: Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341886: Revert [clang-tidy] Handle unresolved 
expressions in ExprMutationAnalyzer (authored by shuaiwang, committed by 
).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51898

Files:
  clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -114,26 +114,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
-TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCode(
-  "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST =
-  tooling::buildASTFromCode("template  void f() { T x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct X;"
-  "template  void f() { X x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-}
-
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
   const auto AST = tooling::buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
@@ -312,51 +292,6 @@
   ElementsAre("std::forward(x)"));
 }
 
-TEST(ExprMutationAnalyzerTest, CallUnresolved) {
-  auto AST =
-  tooling::buildASTFromCode("template  void f() { T x; g(x); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { char x[N]; g(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; g(t, x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(t, x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; t.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("t.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct S;"
-  "template  void f() { S s; int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "struct S { template  void mf(); };"
-  "template  void f(S s) { int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode("template "
-  "void g(F f) { int x; f(x); } ");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { int x; (void)T(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
-}
-
 TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
   const auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
   const auto Results =
@@ -412,21 +347,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
 }
 
-TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
-  const auto AST = tooling::buildASTFromCode(
-  "template  struct S { static constexpr int v = 8; };"
-  "template <> struct S { static constexpr int v = 4; };"
-  "void g(char*);"
-  "template  void f() { char x[S::v]; g(x); }"
-  "template <> void f() { char y[S::v]; g(y); }");
-  const auto ResultsX =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(ResultsX, AST.get()), ElementsAre("g(x)"));
-  const auto ResultsY =
-

[clang-tools-extra] r341886 - Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

2018-09-10 Thread Shuai Wang via cfe-commits 
Author: shuaiwang
Date: Mon Sep 10 16:58:04 2018
New Revision: 341886

URL: http://llvm.org/viewvc/llvm-project?rev=341886=rev
Log:
Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

Summary:
Tests somehow break on windows (and only on windows)
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/13003
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/13747

I have yet figure out why so reverting to unbreak first.

Reviewers: george.karpenkov

Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341886=341885=341886=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Mon Sep 
10 16:58:04 2018
@@ -145,16 +145,11 @@ const Stmt *ExprMutationAnalyzer::findDi
 hasUnaryOperand(equalsNode(Exp)));
 
   // Invoking non-const member function.
-  // A member function is assumed to be non-const when it is unresolved.
   const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
   const auto AsNonConstThis =
   expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), 
on(equalsNode(Exp))),
  cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp))),
- callExpr(callee(expr(anyOf(
- 
unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
- cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp);
+ hasArgument(0, equalsNode(Exp);
 
   // Taking address of 'Exp'.
   // We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -170,16 +165,10 @@ const Stmt *ExprMutationAnalyzer::findDi
unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
 
   // Used as non-const-ref argument when calling a function.
-  // An argument is assumed to be non-const-ref when the function is 
unresolved.
   const auto NonConstRefParam = forEachArgumentWithParam(
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
-  const auto AsNonConstRefArg = anyOf(
-  callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam),
-  callExpr(callee(expr(anyOf(unresolvedLookupExpr(), 
unresolvedMemberExpr(),
- cxxDependentScopeMemberExpr(),
- hasType(templateTypeParmType(),
-   hasAnyArgument(equalsNode(Exp))),
-  cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp;
+  const auto AsNonConstRefArg =
+  anyOf(callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam));
 
   // Captured by a lambda by reference.
   // If we're initializing a capture with 'Exp' directly then we're 
initializing
@@ -206,12 +195,9 @@ const Stmt *ExprMutationAnalyzer::findDi
 
 const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
   // Check whether any member of 'Exp' is mutated.
-  const auto MemberExprs =
-  
match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
-   cxxDependentScopeMemberExpr(
-   hasObjectExpression(equalsNode(Exp)
-.bind("expr")),
-Stm, Context);
+  const auto MemberExprs = match(
+  findAll(memberExpr(hasObjectExpression(equalsNode(Exp))).bind("expr")),
+  Stm, Context);
   return findExprMutation(MemberExprs);
 }
 

Modified: 
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=341886=341885=341886=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
Mon Sep 10 16:58:04 2018
@@ -114,26 +114,6 @@ TEST(ExprMutationAnalyzerTest, NonConstM
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
-TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCode(
-  "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results,

[PATCH] D51898: Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang updated this revision to Diff 164773.
shuaiwang added a comment.

rebase


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51898

Files:
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -114,26 +114,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
-TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCode(
-  "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST =
-  tooling::buildASTFromCode("template  void f() { T x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct X;"
-  "template  void f() { X x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-}
-
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
   const auto AST = tooling::buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
@@ -312,51 +292,6 @@
   ElementsAre("std::forward(x)"));
 }
 
-TEST(ExprMutationAnalyzerTest, CallUnresolved) {
-  auto AST =
-  tooling::buildASTFromCode("template  void f() { T x; g(x); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { char x[N]; g(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; g(t, x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(t, x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; t.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("t.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct S;"
-  "template  void f() { S s; int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "struct S { template  void mf(); };"
-  "template  void f(S s) { int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode("template "
-  "void g(F f) { int x; f(x); } ");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { int x; (void)T(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
-}
-
 TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
   const auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
   const auto Results =
@@ -412,21 +347,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
 }
 
-TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
-  const auto AST = tooling::buildASTFromCode(
-  "template  struct S { static constexpr int v = 8; };"
-  "template <> struct S { static constexpr int v = 4; };"
-  "void g(char*);"
-  "template  void f() { char x[S::v]; g(x); }"
-  "template <> void f() { char y[S::v]; g(y); }");
-  const auto ResultsX =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(ResultsX, AST.get()), ElementsAre("g(x)"));
-  const auto ResultsY =
-  match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
-}
-
 TEST(ExprMutationAnalyzerTest, FollowRefModified) {
   const auto AST = tooling::buildASTFromCode(
   "void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
@@ -478,43 +398,21 @@
 }
 
 TEST(ExprMutationAnalyzerTest, NestedMemberModified) {
-

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

2018-09-10 Thread George Morgan via Phabricator via cfe-commits 
georgemorgan created this revision.
georgemorgan added reviewers: joker-eph-DISABLED, jroelofs, harlanhaskins, 
CodaFi.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, kristina, kristof.beyls.

When building for a baremetal triple an extraneous ".a" is appended to the 
clang_rt.builtins library. This causes the linker to fail to find the static 
library, as the flag passed to the linker is `-lclang_rt.builtins-arm.a` 
instead of `-lclang_rt.builtins-arm`.

  ld.lld: error: unable to find library -lclang_rt.builtins-arm.a

Removing this extraneous '.a' produces the desired behavior and the link 
succeeds. Tests were updated to reflect the change.


Repository:
  rC Clang

https://reviews.llvm.org/D51899

Files:
  lib/Driver/ToolChains/BareMetal.cpp
  test/Driver/baremetal.cpp


Index: test/Driver/baremetal.cpp
===
--- test/Driver/baremetal.cpp
+++ test/Driver/baremetal.cpp
@@ -13,7 +13,7 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" 
"-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -35,7 +35,7 @@
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -48,7 +48,7 @@
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -61,7 +61,7 @@
 // CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBSTDCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
-// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-LIBSTDCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: lib/Driver/ToolChains/BareMetal.cpp
===
--- lib/Driver/ToolChains/BareMetal.cpp
+++ lib/Driver/ToolChains/BareMetal.cpp
@@ -157,7 +157,7 @@
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
   CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName() + ".a"));
+   getTriple().getArchName()));
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,


Index: test/Driver/baremetal.cpp
===
--- test/Driver/baremetal.cpp
+++ test/Driver/baremetal.cpp
@@ -13,7 +13,7 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -35,7 +35,7 @@
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"

[PATCH] D51898: Revert "[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer"

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang created this revision.
Herald added subscribers: cfe-commits, Szelethus, a.sidorin, xazax.hun.
Herald added a reviewer: george.karpenkov.

Tests somehow break on windows (and only on windows)
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/13003
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/13747

I have yet figure out why so reverting to unbreak first.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51898

Files:
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -114,26 +114,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
-TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCode(
-  "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST =
-  tooling::buildASTFromCode("template  void f() { T x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct X;"
-  "template  void f() { X x; x.mf(); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
-}
-
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
   const auto AST = tooling::buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
@@ -312,51 +292,6 @@
   ElementsAre("std::forward(x)"));
 }
 
-TEST(ExprMutationAnalyzerTest, CallUnresolved) {
-  auto AST =
-  tooling::buildASTFromCode("template  void f() { T x; g(x); }");
-  auto Results =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { char x[N]; g(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; g(t, x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(t, x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f(T t) { int x; t.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("t.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  struct S;"
-  "template  void f() { S s; int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "struct S { template  void mf(); };"
-  "template  void f(S s) { int x; s.mf(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
-
-  AST = tooling::buildASTFromCode("template "
-  "void g(F f) { int x; f(x); } ");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f(x)"));
-
-  AST = tooling::buildASTFromCode(
-  "template  void f() { int x; (void)T(x); }");
-  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
-}
-
 TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
   const auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
   const auto Results =
@@ -412,21 +347,6 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
 }
 
-TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
-  const auto AST = tooling::buildASTFromCode(
-  "template  struct S { static constexpr int v = 8; };"
-  "template <> struct S { static constexpr int v = 4; };"
-  "void g(char*);"
-  "template  void f() { char x[S::v]; g(x); }"
-  "template <> void f() { char y[S::v]; g(y); }");
-  const auto ResultsX =
-  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(ResultsX, AST.get()), ElementsAre("g(x)"));
-  const auto ResultsY =
-  match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
-

[PATCH] D51719: Wrapped block after case label should not be merged into a single line (Bug 38854)

2018-09-10 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added a reviewer: krasimir.
owenpan added a comment.

Can one of the reviews have a quick look at this one? Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D51719



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

r341877 - clang-check: rename the local FixItAction

2018-09-10 Thread Saleem Abdulrasool via cfe-commits 
Author: compnerd
Date: Mon Sep 10 15:57:26 2018
New Revision: 341877

URL: http://llvm.org/viewvc/llvm-project?rev=341877=rev
Log:
clang-check: rename the local FixItAction

Resolve the ambiguity in the FixItAction definition by renaming the type.  With
Xcode 9.2, you would fail to build this with:

  llvm/tools/clang/tools/clang-check/ClangCheck.cpp:183:48: error: reference to 
'FixItAction' is ambiguous
  FrontendFactory = newFrontendActionFactory();
 ^

Modified:
cfe/trunk/tools/clang-check/ClangCheck.cpp

Modified: cfe/trunk/tools/clang-check/ClangCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-check/ClangCheck.cpp?rev=341877=341876=341877=diff
==
--- cfe/trunk/tools/clang-check/ClangCheck.cpp (original)
+++ cfe/trunk/tools/clang-check/ClangCheck.cpp Mon Sep 10 15:57:26 2018
@@ -122,7 +122,7 @@ public:
 
 /// Subclasses \c clang::FixItAction so that we can install the custom
 /// \c FixItRewriter.
-class FixItAction : public clang::FixItAction {
+class ClangCheckFixItAction : public clang::FixItAction {
 public:
   bool BeginSourceFileAction(clang::CompilerInstance& CI) override {
 FixItOpts.reset(new FixItOptions);
@@ -180,7 +180,7 @@ int main(int argc, const char **argv) {
   if (Analyze)
 FrontendFactory = newFrontendActionFactory();
   else if (Fixit)
-FrontendFactory = newFrontendActionFactory();
+FrontendFactory = newFrontendActionFactory();
   else
 FrontendFactory = newFrontendActionFactory();
 


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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

2018-09-10 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341874: [Sema][ObjC] Infer availability of +new from 
availability of -init. (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51189?vs=164316=164758#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51189

Files:
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/AST/DeclObjC.h
  cfe/trunk/include/clang/AST/NSAPI.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/DeclObjC.cpp
  cfe/trunk/lib/AST/NSAPI.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/test/SemaObjC/infer-availability-from-init.m

Index: cfe/trunk/lib/AST/NSAPI.cpp
===
--- cfe/trunk/lib/AST/NSAPI.cpp
+++ cfe/trunk/lib/AST/NSAPI.cpp
@@ -607,3 +607,11 @@
   }
   return Sel;
 }
+
+Selector NSAPI::getOrInitNullarySelector(StringRef Id, Selector ) const {
+  if (Sel.isNull()) {
+IdentifierInfo *Ident = (Id);
+Sel = Ctx.Selectors.getSelector(0, );
+  }
+  return Sel;
+}
Index: cfe/trunk/lib/AST/DeclObjC.cpp
===
--- cfe/trunk/lib/AST/DeclObjC.cpp
+++ cfe/trunk/lib/AST/DeclObjC.cpp
@@ -829,6 +829,14 @@
   hasAttr();
 }
 
+bool ObjCMethodDecl::definedInNSObject(const ASTContext ) const {
+  if (const auto *PD = dyn_cast(getDeclContext()))
+return PD->getIdentifier() == Ctx.getNSObjectName();
+  if (const auto *ID = dyn_cast(getDeclContext()))
+return ID->getIdentifier() == Ctx.getNSObjectName();
+  return false;
+}
+
 bool ObjCMethodDecl::isDesignatedInitializerForTheInterface(
 const ObjCMethodDecl **InitMethod) const {
   if (getMethodFamily() != OMF_init)
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -206,7 +206,8 @@
 bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
  const ObjCInterfaceDecl *UnknownObjCClass,
  bool ObjCPropertyAccess,
- bool AvoidPartialAvailabilityChecks) {
+ bool AvoidPartialAvailabilityChecks,
+ ObjCInterfaceDecl *ClassReceiver) {
   SourceLocation Loc = Locs.front();
   if (getLangOpts().CPlusPlus && isa(D)) {
 // If there were any diagnostics suppressed by template argument deduction,
@@ -292,7 +293,7 @@
   }
 
   DiagnoseAvailabilityOfDecl(D, Locs, UnknownObjCClass, ObjCPropertyAccess,
- AvoidPartialAvailabilityChecks);
+ AvoidPartialAvailabilityChecks, ClassReceiver);
 
   DiagnoseUnusedOfDecl(*this, D, Loc);
 
Index: cfe/trunk/lib/Sema/SemaExprObjC.cpp
===
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp
@@ -2471,7 +2471,8 @@
 if (!Method)
   Method = Class->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs,
+nullptr, false, false, Class))
   return ExprError();
   }
 
@@ -2784,14 +2785,19 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
+// FIXME: Is this correct? Why are we assuming that a message to
+// Class will call a method in the current interface?
+
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
 
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
+
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
+false, false, ClassDecl))
+  return ExprError();
   }
-  if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
-return ExprError();
 }
 if (!Method) {
   // If not messaging 'self', look for any factory method named 'Sel'.
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -6967,8 +6967,12 @@
 /// \param D The declaration to check.
 /// \param Message If non-null, this will be populated with the message from
 /// the availability attribute that is selected.
+/// \param ClassReceiver If we're checking the the method of a class message
+/// send, the class. Otherwise nullptr.
 static std::pair
-ShouldDiagnoseAvailabilityOfDecl(const NamedDecl *D, std::string *Message) {

r341874 - [Sema][ObjC] Infer availability of +new from availability of -init.

2018-09-10 Thread Erik Pilkington via cfe-commits 
Author: epilk
Date: Mon Sep 10 15:20:09 2018
New Revision: 341874

URL: http://llvm.org/viewvc/llvm-project?rev=341874=rev
Log:
[Sema][ObjC] Infer availability of +new from availability of -init.

When defined in NSObject, +new will call -init. If -init has been marked
unavailable, diagnose uses of +new.

rdar://18335828

Differential revision: https://reviews.llvm.org/D51189

Added:
cfe/trunk/test/SemaObjC/infer-availability-from-init.m
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/include/clang/AST/NSAPI.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=341874=341873=341874=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Mon Sep 10 15:20:09 2018
@@ -336,7 +336,7 @@ private:
   mutable IdentifierInfo *BoolName = nullptr;
 
   /// The identifier 'NSObject'.
-  IdentifierInfo *NSObjectName = nullptr;
+  mutable IdentifierInfo *NSObjectName = nullptr;
 
   /// The identifier 'NSCopying'.
   IdentifierInfo *NSCopyingName = nullptr;
@@ -1676,7 +1676,7 @@ public:
   }
 
   /// Retrieve the identifier 'NSObject'.
-  IdentifierInfo *getNSObjectName() {
+  IdentifierInfo *getNSObjectName() const {
 if (!NSObjectName) {
   NSObjectName = ("NSObject");
 }

Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=341874=341873=341874=diff
==
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Mon Sep 10 15:20:09 2018
@@ -506,6 +506,9 @@ public:
   /// Returns whether this specific method is a definition.
   bool isThisDeclarationADefinition() const { return hasBody(); }
 
+  /// Is this method defined in the NSObject base class?
+  bool definedInNSObject(const ASTContext &) const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == ObjCMethod; }

Modified: cfe/trunk/include/clang/AST/NSAPI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/NSAPI.h?rev=341874=341873=341874=diff
==
--- cfe/trunk/include/clang/AST/NSAPI.h (original)
+++ cfe/trunk/include/clang/AST/NSAPI.h Mon Sep 10 15:20:09 2018
@@ -166,6 +166,14 @@ public:
 return getOrInitSelector(StringRef("isEqual"), isEqualSel);
   }
 
+  Selector getNewSelector() const {
+return getOrInitNullarySelector("new", NewSel);
+  }
+
+  Selector getInitSelector() const {
+return getOrInitNullarySelector("init", InitSel);
+  }
+
   /// Enumerates the NSNumber methods used to generate literals.
   enum NSNumberLiteralMethodKind {
 NSNumberWithChar,
@@ -229,6 +237,7 @@ private:
   bool isObjCEnumerator(const Expr *E,
 StringRef name, IdentifierInfo *) const;
   Selector getOrInitSelector(ArrayRef Ids, Selector ) const;
+  Selector getOrInitNullarySelector(StringRef Id, Selector ) const;
 
   ASTContext 
 
@@ -251,7 +260,7 @@ private:
 
   mutable Selector objectForKeyedSubscriptSel, objectAtIndexedSubscriptSel,

setObjectForKeyedSubscriptSel,setObjectAtIndexedSubscriptSel,
-   isEqualSel;
+   isEqualSel, InitSel, NewSel;
 
   mutable IdentifierInfo *BOOLId, *NSIntegerId, *NSUIntegerId;
   mutable IdentifierInfo *NSASCIIStringEncodingId, *NSUTF8StringEncodingId;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=341874=341873=341874=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Sep 10 15:20:09 2018
@@ -3967,7 +3967,8 @@ public:
   void DiagnoseAvailabilityOfDecl(NamedDecl *D, ArrayRef Locs,
   const ObjCInterfaceDecl *UnknownObjCClass,
   bool ObjCPropertyAccess,
-  bool AvoidPartialAvailabilityChecks = false);
+  bool AvoidPartialAvailabilityChecks = false,
+  ObjCInterfaceDecl *ClassReceiver = nullptr);
 
   bool makeUnavailableInSystemHeader(SourceLocation loc,
  UnavailableAttr::ImplicitReason reason);
@@ -3982,7 +3983,8 @@ public:
   bool

r341871 - Fix test regression in r341862

2018-09-10 Thread Erich Keane via cfe-commits 
Author: erichkeane
Date: Mon Sep 10 14:57:53 2018
New Revision: 341871

URL: http://llvm.org/viewvc/llvm-project?rev=341871=rev
Log:
Fix test regression in r341862

The commit updates when AES is enabled, but failed to update the tests.
This patch fixes them.

Modified:
cfe/trunk/test/CodeGen/attr-target-x86.c
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/test/CodeGen/attr-target-x86.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-x86.c?rev=341871=341870=341871=diff
==
--- cfe/trunk/test/CodeGen/attr-target-x86.c (original)
+++ cfe/trunk/test/CodeGen/attr-target-x86.c Mon Sep 10 14:57:53 2018
@@ -49,7 +49,7 @@ int __attribute__((target("arch=lakemont
 // CHECK: lake{{.*}} #7
 // CHECK: use_before_def{{.*}} #7
 // CHECK: #0 = {{.*}}"target-cpu"="i686" "target-features"="+x87"
-// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" 
"target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
+// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" 
"target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
 // CHECK: #2 = {{.*}}"target-cpu"="i686" 
"target-features"="+x87,-aes,-avx,-avx2,-avx512bitalg,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vbmi2,-avx512vl,-avx512vnni,-avx512vpopcntdq,-f16c,-fma,-fma4,-gfni,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-vaes,-vpclmulqdq,-xop,-xsave,-xsaveopt"
 // CHECK: #3 = {{.*}}"target-cpu"="i686" 
"target-features"="+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
 // CHECK: #4 = {{.*}}"target-cpu"="i686" 
"target-features"="+x87,-avx,-avx2,-avx512bitalg,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vbmi2,-avx512vl,-avx512vnni,-avx512vpopcntdq,-f16c,-fma,-fma4,-sse4.1,-sse4.2,-vaes,-vpclmulqdq,-xop,-xsave,-xsaveopt"

Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=341871=341870=341871=diff
==
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Mon Sep 10 14:57:53 
2018
@@ -417,7 +417,6 @@
 // RUN: %clang -march=corei7-avx -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_COREI7_AVX_M32
-// CHECK_COREI7_AVX_M32: #define __AES__ 1
 // CHECK_COREI7_AVX_M32: #define __AVX__ 1
 // CHECK_COREI7_AVX_M32: #define __MMX__ 1
 // CHECK_COREI7_AVX_M32: #define __PCLMUL__ 1
@@ -441,7 +440,6 @@
 // RUN: %clang -march=corei7-avx -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_COREI7_AVX_M64
-// CHECK_COREI7_AVX_M64: #define __AES__ 1
 // CHECK_COREI7_AVX_M64: #define __AVX__ 1
 // CHECK_COREI7_AVX_M64: #define __MMX__ 1
 // CHECK_COREI7_AVX_M64: #define __PCLMUL__ 1
@@ -468,7 +466,6 @@
 // RUN: %clang -march=core-avx-i -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_CORE_AVX_I_M32
-// CHECK_CORE_AVX_I_M32: #define __AES__ 1
 // CHECK_CORE_AVX_I_M32: #define __AVX__ 1
 // CHECK_CORE_AVX_I_M32: #define __F16C__ 1
 // CHECK_CORE_AVX_I_M32: #define __MMX__ 1
@@ -492,7 +489,6 @@
 // RUN: %clang -march=core-avx-i -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_CORE_AVX_I_M64
-// CHECK_CORE_AVX_I_M64: #define __AES__ 1
 // CHECK_CORE_AVX_I_M64: #define __AVX__ 1
 // CHECK_CORE_AVX_I_M64: #define __F16C__ 1
 // CHECK_CORE_AVX_I_M64: #define __MMX__ 1
@@ -519,7 +515,6 @@
 // RUN: %clang -march=core-avx2 -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_CORE_AVX2_M32
-// CHECK_CORE_AVX2_M32: #define __AES__ 1
 // CHECK_CORE_AVX2_M32: #define __AVX2__ 1
 // CHECK_CORE_AVX2_M32: #define __AVX__ 1
 // CHECK_CORE_AVX2_M32: #define __BMI2__ 1
@@ -550,7 +545,6 @@
 // RUN: %clang -march=core-avx2 -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_CORE_AVX2_M64
-// CHECK_CORE_AVX2_M64: #define __AES__ 1
 // CHECK_CORE_AVX2_M64: #define __AVX2__ 1
 // CHECK_CORE_AVX2_M64: #define __AVX__ 1
 // CHECK_CORE_AVX2_M64: #define __BMI2__ 1
@@ -585,7 +579,6 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_BROADWELL_M32
 // CHECK_BROADWELL_M32: #define __ADX__ 1
-// CHECK_BROADWELL_M32:

[PATCH] D51867: [Diagnostics] Add error handling to FormatDiagnostic()

2018-09-10 Thread Jan Korous via Phabricator via cfe-commits 
jkorous added a comment.

I tried to come up with some input that breaks current implementation so I 
could add the test. Problem is that invalid memory read doesn't guarantee 
deterministic crash.  
E. g. with this input the current implementation is definitely reading way past 
the buffer:

  SmallVector IgnoreMe;
  const char* Foo = "foo%";
  const char* FooEnd = Foo + 4;
  Diag.FormatDiagnostic(Foo, FooEnd, IgnoreMe);

...and it actually found some string there yet it didn't crash until it hit 
some unrelated assert

  (lldb) p DiagStr
  (const char *) $0 = 0x000100adc53b " SplatSizeInBits == 0 && 
\"SplatSizeInBits must divide width!\""
  (lldb) p *DiagStr
  (const char) $1 = ' '
  (lldb) p DiagEnd
  (const char *) $2 = 0x000100ad4155 "0"

The only reliable fail is passing nullptr which currently leads to SIGABRT 
(nullptr dereferenced)

  SmallVector IgnoreMe;
  const char* Foo = "foo";
  Diag.FormatDiagnostic(Foo, nullptr, IgnoreMe);

I am reconsidering the necessity of such tests here. WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D51867



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

r341869 - [AST] Fix a crash on invalid.

2018-09-10 Thread Erik Pilkington via cfe-commits 
Author: epilk
Date: Mon Sep 10 14:54:04 2018
New Revision: 341869

URL: http://llvm.org/viewvc/llvm-project?rev=341869=rev
Log:
[AST] Fix a crash on invalid.

Problem was that we were appending to the source location info buffer in the
copy assignment operator (instead of overwriting).

rdar://42746401

Added:
cfe/trunk/test/SemaCXX/rdar42746401.cpp
Modified:
cfe/trunk/lib/AST/NestedNameSpecifier.cpp

Modified: cfe/trunk/lib/AST/NestedNameSpecifier.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/NestedNameSpecifier.cpp?rev=341869=341868=341869=diff
==
--- cfe/trunk/lib/AST/NestedNameSpecifier.cpp (original)
+++ cfe/trunk/lib/AST/NestedNameSpecifier.cpp Mon Sep 10 14:54:04 2018
@@ -547,6 +547,7 @@ operator=(const NestedNameSpecifierLocBu
   }
 
   // Deep copy.
+  BufferSize = 0;
   Append(Other.Buffer, Other.Buffer + Other.BufferSize, Buffer, BufferSize,
  BufferCapacity);
   return *this;

Added: cfe/trunk/test/SemaCXX/rdar42746401.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/rdar42746401.cpp?rev=341869=auto
==
--- cfe/trunk/test/SemaCXX/rdar42746401.cpp (added)
+++ cfe/trunk/test/SemaCXX/rdar42746401.cpp Mon Sep 10 14:54:04 2018
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template 
+class b;
+class c; // expected-note{{forward declaration}}
+
+::b<0> struct c::d // expected-error{{incomplete type}} expected-error{{cannot 
combine}} expected-error{{expected unqualified-id}}


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

[PATCH] D51510: Move AESNI generation to Skylake and Goldmont

2018-09-10 Thread Erich Keane via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341862: Move AESNI generation to Skylake and Goldmont 
(authored by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51510?vs=163415=164750#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51510

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -170,6 +170,7 @@
   setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "rtm", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Broadwell:
 setFeatureEnabledImpl(Features, "rdseed", true);
@@ -196,7 +197,6 @@
 setFeatureEnabledImpl(Features, "xsaveopt", true);
 LLVM_FALLTHROUGH;
   case CK_Westmere:
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 LLVM_FALLTHROUGH;
   case CK_Nehalem:
@@ -248,10 +248,10 @@
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "mpx", true);
 setFeatureEnabledImpl(Features, "fsgsbase", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Silvermont:
 setFeatureEnabledImpl(Features, "rdrnd", true);
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 setFeatureEnabledImpl(Features, "sse4.2", true);
 setFeatureEnabledImpl(Features, "prfchw", true);


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -170,6 +170,7 @@
   setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "rtm", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Broadwell:
 setFeatureEnabledImpl(Features, "rdseed", true);
@@ -196,7 +197,6 @@
 setFeatureEnabledImpl(Features, "xsaveopt", true);
 LLVM_FALLTHROUGH;
   case CK_Westmere:
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 LLVM_FALLTHROUGH;
   case CK_Nehalem:
@@ -248,10 +248,10 @@
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "mpx", true);
 setFeatureEnabledImpl(Features, "fsgsbase", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Silvermont:
 setFeatureEnabledImpl(Features, "rdrnd", true);
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 setFeatureEnabledImpl(Features, "sse4.2", true);
 setFeatureEnabledImpl(Features, "prfchw", true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r341862 - Move AESNI generation to Skylake and Goldmont

2018-09-10 Thread Erich Keane via cfe-commits 
Author: erichkeane
Date: Mon Sep 10 14:12:21 2018
New Revision: 341862

URL: http://llvm.org/viewvc/llvm-project?rev=341862=rev
Log:
Move AESNI generation to Skylake and Goldmont

The instruction set first appeared with Westmere, but not all processors
in that and the next few generations have the instructions. According to
Wikipedia[1], the first generation in which all SKUs have AES
instructions are Skylake and Goldmont. I can't find any Skylake,
Kabylake, Kabylake-R or Cannon Lake currently listed at
https://ark.intel.com that says "Intel® AES New Instructions" "No".

This matches GCC commit
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01940.html

[1] https://en.wikipedia.org/wiki/AES_instruction_set

Patch By: thiagomacieira
Differential Revision: https://reviews.llvm.org/D51510

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=341862=341861=341862=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Sep 10 14:12:21 2018
@@ -170,6 +170,7 @@ bool X86TargetInfo::initFeatureMap(
   setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "rtm", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Broadwell:
 setFeatureEnabledImpl(Features, "rdseed", true);
@@ -196,7 +197,6 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "xsaveopt", true);
 LLVM_FALLTHROUGH;
   case CK_Westmere:
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 LLVM_FALLTHROUGH;
   case CK_Nehalem:
@@ -248,10 +248,10 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "mpx", true);
 setFeatureEnabledImpl(Features, "fsgsbase", true);
+setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;
   case CK_Silvermont:
 setFeatureEnabledImpl(Features, "rdrnd", true);
-setFeatureEnabledImpl(Features, "aes", true);
 setFeatureEnabledImpl(Features, "pclmul", true);
 setFeatureEnabledImpl(Features, "sse4.2", true);
 setFeatureEnabledImpl(Features, "prfchw", true);


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

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

2018-09-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
vsapsai added a comment.

Agree that fatal/non-fatal error is too coarse and tooling/IDEs need more 
details and more control to provide better experience. But I don't think we are 
in a state to claim that all errors are recoverable (in theory and in current 
implementation). Instead of continuing on all errors, I prefer to select errors 
that are important for tooling and improve those first.

Regarding the current patch, I don't like creating coupling between 
`hasFatalErrorOccurred` and `shouldRecoverAfterFatalErrors`. Looks like after 
this patch you'll need to call these methods together in many cases. For 
example, probably `Sema::makeTypoCorrectionConsumer` in

  if (Diags.hasFatalErrorOccurred() || !getLangOpts().SpellChecking ||
  DisableTypoCorrection)
return nullptr;

should check `shouldRecoverAfterFatalErrors` too.


Repository:
  rC Clang

https://reviews.llvm.org/D50462



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

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

2018-09-10 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D51189



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

[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client

2018-09-10 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

In https://reviews.llvm.org/D50814#1224292, @ilya-biryukov wrote:

> Sorry for the delayed response. It seems we absolutely need this if mirroring 
>  libclang is an absolute requirement.
>  We seem to have multiple implementation options:
>
> Which classes to use for representing diagnostics? We can:
>
> 1. Reuse existing hierarchy for diagnostics. (current option)
> 2. Have separate hierarchies for "flat" (fixits with notes) and "grouped" 
> (fix-its are always attached to main diag). "flat" diagnostics can be stored 
> exactly as they come from clang, with notes and main diags in the same 
> vector, main diag should lack the "notes" field. I.e. if we choose to go with 
> "raw" clang mode, we can as well avoid grouping the notes altogether instead 
> of having code that does something in between the classical libclang and new 
> clangd approach.
>
>   I think the added separation is worth it for code clarity, but also realize 
> it's a bunch of boilerplate, so others may disagree. WDYT?


I think having separate hierarchies would be more confusing and harder to test 
well. I'd prefer to use one hierarchy.

> Another alternative that we might explore is always producing fix-its 
> attached to notes at the lowest level (in `Diagnostics.cpp`) and 
> reconstructing the current diagnostics hierarchy somewhere higher up the 
> chain, e.g. at the LSPServer level.
>  This would allow to keep the option at the LSPServer level and would only 
> require extracting a (seemingly simple) conversion function that can be 
> called at the LSP Server level.
>  Having the option handled at the top-level seems like a win, since most of 
> the code does not need to care about this option.\

That would be my personal preference. I will work on an updated patch that does 
that.

> +@sammccall, who wanted to take a look at this issue.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50814



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

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

2018-09-10 Thread JF Bastien via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341860: Implement -Watomic-implicit-seq-cst (authored by 
jfb, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51084?vs=164235=164746#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51084

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/atomic-implicit-seq_cst.c
  test/Sema/sync-implicit-seq_cst.c

Index: test/Sema/atomic-implicit-seq_cst.c
===
--- test/Sema/atomic-implicit-seq_cst.c
+++ test/Sema/atomic-implicit-seq_cst.c
@@ -0,0 +1,325 @@
+// RUN: %clang_cc1 %s -verify -ffreestanding -fsyntax-only -triple=i686-linux-gnu -std=c11 -Watomic-implicit-seq-cst
+
+// _Atomic operations are implicitly sequentially-consistent. Some codebases
+// want to force explicit usage of memory order instead.
+
+_Atomic(int) atom;
+void gimme_int(int);
+
+void bad_pre_inc(void) {
+  ++atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+void bad_pre_dec(void) {
+  --atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+void bad_post_inc(void) {
+  atom++; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+void bad_post_dec(void) {
+  atom--; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+void bad_call(void) {
+  gimme_int(atom); // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_unary_plus(void) {
+  return +atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_unary_minus(void) {
+  return -atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_unary_logical_not(void) {
+  return !atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_unary_bitwise_not(void) {
+  return ~atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_explicit_cast(void) {
+  return (int)atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_implicit_cast(void) {
+  return atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_mul_1(int i) {
+  return atom * i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_mul_2(int i) {
+  return i * atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_div_1(int i) {
+  return atom / i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_div_2(int i) {
+  return i / atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_mod_1(int i) {
+  return atom % i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_mod_2(int i) {
+  return i % atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_add_1(int i) {
+  return atom + i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_add_2(int i) {
+  return i + atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_sub_1(int i) {
+  return atom - i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_sub_2(int i) {
+  return i - atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_shl_1(int i) {
+  return atom << i; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_shl_2(int i) {
+  return i << atom; // expected-warning {{implicit use of sequentially-consistent atomic may incur stronger memory barriers than necessary}}
+}
+
+int bad_shr_1(int i) {
+  return atom >> i; // expected-warning {{implicit use of sequentially-consistent

r341860 - Implement -Watomic-implicit-seq-cst

2018-09-10 Thread JF Bastien via cfe-commits 
Author: jfb
Date: Mon Sep 10 13:42:56 2018
New Revision: 341860

URL: http://llvm.org/viewvc/llvm-project?rev=341860=rev
Log:
Implement -Watomic-implicit-seq-cst

Summary:
_Atomic and __sync_* operations are implicitly sequentially-consistent. Some
codebases want to force explicit usage of memory order instead. This warning
allows them to know where implicit sequentially-consistent memory order is used.
The warning isn't on by default because _Atomic was purposefully designed to
have seq_cst as the default: the idea was that it's the right thing to use most
of the time. This warning allows developers who disagree to enforce explicit
usage instead.

A follow-up patch will take care of C++'s std::atomic. It'll be different enough
from this patch that I think it should be separate: for C++ the atomic
operations all have a memory order parameter (or two), but it's defaulted. I
believe this warning should trigger when the default is used, but not when
seq_cst is used explicitly (or implicitly as the failure order for cmpxchg).



Reviewers: rjmccall

Subscribers: dexonsmith, cfe-commits

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

Added:
cfe/trunk/test/Sema/atomic-implicit-seq_cst.c
cfe/trunk/test/Sema/sync-implicit-seq_cst.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341860=341859=341860=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 10 13:42:56 
2018
@@ -7096,6 +7096,9 @@ def err_atomic_op_has_invalid_synch_scop
 def warn_atomic_op_misaligned : Warning<
   "%select{large|misaligned}0 atomic operation may incur "
   "significant performance penalty">, InGroup>;
+def warn_atomic_implicit_seq_cst : Warning<
+  "implicit use of sequentially-consistent atomic may incur stronger memory 
barriers than necessary">,
+  InGroup>, DefaultIgnore;
 
 def err_overflow_builtin_must_be_int : Error<
   "operand argument to overflow builtin must be an integer (%0 invalid)">;

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=341860=341859=341860=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Sep 10 13:42:56 2018
@@ -1134,6 +1134,10 @@ Sema::CheckBuiltinFunctionCall(FunctionD
   case Builtin::BI__sync_swap_8:
   case Builtin::BI__sync_swap_16:
 return SemaBuiltinAtomicOverloaded(TheCallResult);
+  case Builtin::BI__sync_synchronize:
+Diag(TheCall->getBeginLoc(), diag::warn_atomic_implicit_seq_cst)
+<< TheCall->getCallee()->getSourceRange();
+break;
   case Builtin::BI__builtin_nontemporal_load:
   case Builtin::BI__builtin_nontemporal_store:
 return SemaBuiltinNontemporalOverloaded(TheCallResult);
@@ -4646,25 +4650,24 @@ static bool checkBuiltinArgument(Sema 
   return false;
 }
 
-/// SemaBuiltinAtomicOverloaded - We have a call to a function like
-/// __sync_fetch_and_add, which is an overloaded function based on the pointer
-/// type of its first argument.  The main ActOnCallExpr routines have already
-/// promoted the types of arguments because all of these calls are prototyped 
as
-/// void(...).
+/// We have a call to a function like __sync_fetch_and_add, which is an
+/// overloaded function based on the pointer type of its first argument.
+/// The main ActOnCallExpr routines have already promoted the types of
+/// arguments because all of these calls are prototyped as void(...).
 ///
 /// This function goes through and does final semantic checking for these
-/// builtins,
+/// builtins, as well as generating any warnings.
 ExprResult
 Sema::SemaBuiltinAtomicOverloaded(ExprResult TheCallResult) {
-  CallExpr *TheCall = (CallExpr *)TheCallResult.get();
-  DeclRefExpr *DRE 
=cast(TheCall->getCallee()->IgnoreParenCasts());
+  CallExpr *TheCall = static_cast(TheCallResult.get());
+  Expr *Callee = TheCall->getCallee();
+  DeclRefExpr *DRE = cast(Callee->IgnoreParenCasts());
   FunctionDecl *FDecl = cast(DRE->getDecl());
 
   // Ensure that we have at least one argument to do type inference from.
   if (TheCall->getNumArgs() < 1) {
 Diag(TheCall->getEndLoc(), diag::err_typecheck_call_too_few_args_at_least)
-<< 0 << 1 << TheCall->getNumArgs()
-<< TheCall->getCallee()->getSourceRange();
+<< 0 << 1 << TheCall->getNumArgs() << Callee->getSourceRange();
 return ExprError();
   }
 
@@ -4941,13 +4944,16 @@ Sema::SemaBuiltinAtomicOverloaded(ExprRe
   if (TheCall->getNumArgs() < 1+NumFixed) {
 Diag(TheCall->getEndLoc(),

[PATCH] D51650: Implement target_clones multiversioning

2018-09-10 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:961-970
   if (const auto *FD = dyn_cast(ND))
 if (FD->isMultiVersion() && !OmitMultiVersionMangling) {
   if (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
 AppendCPUSpecificCPUDispatchMangling(
 CGM, FD->getAttr(), Out);
+  else if (FD->isTargetClonesMultiVersion())
+AppendTargetClonesMangling(CGM, FD->getAttr(), Out);

erichkeane wrote:
> rsmith wrote:
> > This chain of `if`s and similar things elsewhere make me think we're 
> > missing an abstraction. Perhaps `FunctionDecl` should have a 
> > `getMultiVersionAttr()` and/or `getMultiVersionKind()` functions?
> I'm not super sure what either buys us... The multiversion attributes are all 
> somewhat different unfortunately, so they would need to be dispatched 
> separately later.  The 'getMultiVersionKind' is perhaps useful, though its 
> essentially what isXXXMultiVersion does.  I'll think on it, I agree that 
> there is likely an abstraction somewhere between that can improve this...
The idea would be that we have an enum that we can perform a covered `switch` 
over, so we don't need to remember to update all the relevant places when we 
add another kind of multiversioning. You already have the code to work out the 
kind of multiversioning; moving it from SemaDecl.cpp to FunctionDecl then using 
it where it makes sense in this patch would help, I think.



Comment at: lib/Sema/SemaDecl.cpp:9811-9812
 
   // Main isn't allowed to become a multiversion function, however it IS
   // permitted to have 'main' be marked with the 'target' optimization hint.
   if (NewFD->isMain()) {

erichkeane wrote:
> rsmith wrote:
> > Why can't `main` be multiversioned? That seems like an arbitrary 
> > restriction.
> At the time of implementing 'target', I was unsure of (and still am) how to 
> accomplish this.  It would seem that I'd need to make the entry point a 
> wrapper that calls the ifunc.  GCC seems to improperly call this (it doesn't 
> emit the 'main' fucntion as far as I can tell).
OK, if there's actually a problem with having the `main` symbol be an ifunc, 
that seems like a perfectly legitimate reason for the restriction.


https://reviews.llvm.org/D51650



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

r341858 - Enhance -Wc++14-compat for class template argument deduction to list the

2018-09-10 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Mon Sep 10 13:31:03 2018
New Revision: 341858

URL: http://llvm.org/viewvc/llvm-project?rev=341858=rev
Log:
Enhance -Wc++14-compat for class template argument deduction to list the
deduced type (if known).

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/SemaCXX/cxx14-compat.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=341858=341857=341858=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 10 13:31:03 
2018
@@ -2153,7 +2153,8 @@ def note_deduction_guide_access : Note<
   "deduction guide declared %0 by intervening access specifier">;
 def warn_cxx14_compat_class_template_argument_deduction : Warning<
   "class template argument deduction is incompatible with C++ standards "
-  "before C++17">, InGroup, DefaultIgnore;
+  "before C++17%select{|; for compatibility, use explicit type name %1}0">,
+  InGroup, DefaultIgnore;
 
 // C++14 deduced return types
 def err_auto_fn_deduction_failure : Error<

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=341858=341857=341858=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Sep 10 13:31:03 2018
@@ -9139,13 +9139,13 @@ QualType Sema::DeduceTemplateSpecializat
 return QualType();
   }
 
-  Diag(TSInfo->getTypeLoc().getBeginLoc(),
-   diag::warn_cxx14_compat_class_template_argument_deduction)
-  << TSInfo->getTypeLoc().getSourceRange();
-
   // Can't deduce from dependent arguments.
-  if (Expr::hasAnyTypeDependentArguments(Inits))
+  if (Expr::hasAnyTypeDependentArguments(Inits)) {
+Diag(TSInfo->getTypeLoc().getBeginLoc(),
+ diag::warn_cxx14_compat_class_template_argument_deduction)
+<< TSInfo->getTypeLoc().getSourceRange() << 0;
 return Context.DependentTy;
+  }
 
   // FIXME: Perform "exact type" matching first, per CWG discussion?
   //Or implement this via an implied 'T(T) -> T' deduction guide?
@@ -9348,5 +9348,10 @@ QualType Sema::DeduceTemplateSpecializat
   // C++ [dcl.type.class.deduct]p1:
   //  The placeholder is replaced by the return type of the function selected
   //  by overload resolution for class template deduction.
-  return SubstAutoType(TSInfo->getType(), Best->Function->getReturnType());
+  QualType DeducedType =
+  SubstAutoType(TSInfo->getType(), Best->Function->getReturnType());
+  Diag(TSInfo->getTypeLoc().getBeginLoc(),
+   diag::warn_cxx14_compat_class_template_argument_deduction)
+  << TSInfo->getTypeLoc().getSourceRange() << 1 << DeducedType;
+  return DeducedType;
 }

Modified: cfe/trunk/test/SemaCXX/cxx14-compat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx14-compat.cpp?rev=341858=341857=341858=diff
==
--- cfe/trunk/test/SemaCXX/cxx14-compat.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx14-compat.cpp Mon Sep 10 13:31:03 2018
@@ -16,7 +16,7 @@ namespace [[]] NS_with_attr {} // expect
 enum { e [[]] }; // expected-warning {{incompatible with C++ standards before 
C++17}}
 
 template struct X {};
-X x; // expected-warning {{class template argument deduction is incompatible 
with C++ standards before C++17}}
+X x; // expected-warning {{class template argument deduction is incompatible 
with C++ standards before C++17; for compatibility, use explicit type name 
'X'}}
 
 template class> struct Y {};
 Y yx; // ok, not class template argument deduction


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

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

Ofc the current limitation with assuming always modification stays with my 
proposed tests. But these are the tests we need once implemented full analysis 
of pointers.




Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:658
+ "void f() { UniquePtr x; x->mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));

Template testcases i miss:

```
// modifying
template 
void f() { UnqiuePtr x; x->mf(); }

// constant
template 
void f2() { UnqiuePtr x; x->cmf(); }

// indecidable for the template itself, but only the instantiations
template 
void f3() { T x; x->cmf(); }

struct const_class { void cmf() const; }
struct modifying_class { void cmf(); };

void call_template() {
// don't trigger
f3>();
// trigger modification
f3();
}

// again not decidable by the template itself
template 
void f4() { T t; *t; }

struct very_weird {
int& operator*() { return *new int(42); }
};
void call_template_deref() {
  // no modification
  f4();
  // modification, because deref is not const
  f4>():
}
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50883



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

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

2018-09-10 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Missing tests and changes to Registry.cpp for dynamic matchers.

Also, do you want to add `isInstantiationDependent()` at the same time, given 
the relationship with the other two matchers?




Comment at: include/clang/ASTMatchers/ASTMatchers.h:777
 
+/// Matches expressions that are type-dependant because the template type
+/// is not yet instantiated.

s/dependant/dependent



Comment at: include/clang/ASTMatchers/ASTMatchers.h:787
+///   }
+/// \endcode
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }

Your example should include the matcher syntax.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:790
+
+/// Matches expression that are value-dependant because they contain a
+/// non-type template parameter.

s/dependant/dependent


Repository:
  rC Clang

https://reviews.llvm.org/D51880



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

[PATCH] D51853: Merge two attribute diagnostics into one

2018-09-10 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D51853



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

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang added a comment.

In https://reviews.llvm.org/D50883#1229297, @JonasToth wrote:

> >   Different from std::vector::operator[] which has two overloads for const 
> > and non-const access, std::unique_ptr only has one const version of 
> > `operator->`.
> > 
> > So for SmartPtr x; x->mf(); we only see a const operator being invoked on 
> > x. mf is not a member of SmartPtr and the member call to mf is not on x 
> > directly, we never followed that far.
>
> I think the `operator->` is transitively called, isn't it? (see andrei 
> alexandrescu talk here: https://youtu.be/ozOgzlxIsdg?t=15m55s where he gives 
> a nice class for generic locking)
>  Maybe there is something more general at work? I think @aaron.ballman knows 
> more about deeper language questions :)


Right, for `x->mf()` (where `x` is some kind of smart pointer), `operator->` is 
invoked on the smart pointer itself, and then that yields a (real) pointer, in 
which case the `operator->` magically reappears and is applied on the real 
pointer.
For now we're basically treating `SmartPtr::operator->()` the same as "taking 
the address of `Exp`", which is treated as immediately mutating `Exp`. The 
benefit of this approach is: when we're able to do `findPointeeMutation`, we'll 
be able to further follow the pointer and see whether the pointee is mutated, 
and we'll be able to distinguish between these two cases:

  struct Foo {
  void mf();
  void cmf() const;
  };
  std::unique_ptr p;
  p->mf(); // mutates Foo
  p->cmf(); // doesn't mutate Foo, but is treated as mutation for now. Can be 
improved when we're able to do findPointeeMutation( () call on p> )


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50883



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

I think you dont need tests there. Its just life insurance :)

I'd stick with pointers because working with the AST always results in
pointers. It is more convenient to stay in pointerland there.

>> I feel that the `findMutation...` functions that take raw pointers should 
>> get the assertions though, at least the ones in the public interface. Having 
>> them for the private ones won't hurt either.
> 
> I could do that.
>  But should that come with tests?
>  Or do we want to also take references there?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884



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

[clang-tools-extra] r341854 - [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Roman Lebedev via cfe-commits 
Author: lebedevri
Date: Mon Sep 10 12:59:18 2018
New Revision: 341854

URL: http://llvm.org/viewvc/llvm-project?rev=341854=rev
Log:
[clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR3

Summary:
I have hit this the rough way, while trying to use this in D51870.

There is no particular point in storing the pointers, and moreover
the pointers are assumed to be non-null, and that assumption is not
enforced. If they are null, it won't be able to do anything good
with them anyway.

Initially i thought about simply adding asserts() that they are
not null, but taking/storing references looks like even cleaner solution?

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=3 | PR3 ]]

Reviewers: JonasToth, shuaiwang, alexfh, george.karpenkov

Reviewed By: shuaiwang

Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.h
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp?rev=341854=341853=341854=diff
==
--- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Mon 
Sep 10 12:59:18 2018
@@ -88,8 +88,8 @@ bool ForRangeCopyCheck::handleCopyIsOnly
   // Because the fix (changing to `const auto &`) will introduce an unused
   // compiler warning which can't be suppressed.
   // Since this case is very rare, it is safe to ignore it.
-  if (!utils::ExprMutationAnalyzer(ForRange.getBody(), )
-  .isMutated() &&
+  if (!utils::ExprMutationAnalyzer(*ForRange.getBody(), Context)
+   .isMutated() &&
   !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
  Context)
.empty()) {

Modified: 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp?rev=341854=341853=341854=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp 
Mon Sep 10 12:59:18 2018
@@ -95,14 +95,14 @@ void UnnecessaryValueParamCheck::check(c
   // Do not trigger on non-const value parameters when they are mutated either
   // within the function body or within init expression(s) when the function is
   // a ctor.
-  if (utils::ExprMutationAnalyzer(Function->getBody(), Result.Context)
+  if (utils::ExprMutationAnalyzer(*Function->getBody(), *Result.Context)
   .isMutated(Param))
 return;
   // CXXCtorInitializer might also mutate Param but they're not part of 
function
   // body, so check them separately here.
   if (const auto *Ctor = dyn_cast(Function)) {
 for (const auto *Init : Ctor->inits()) {
-  if (utils::ExprMutationAnalyzer(Init->getInit(), Result.Context)
+  if (utils::ExprMutationAnalyzer(*Init->getInit(), *Result.Context)
   .isMutated(Param))
 return;
 }

Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341854=341853=341854=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Mon Sep 
10 12:59:18 2018
@@ -102,7 +102,7 @@ bool ExprMutationAnalyzer::isUnevaluated
   hasDescendant(equalsNode(Exp,
   cxxNoexceptExpr())
  .bind("expr")),
- *Stm, *Context)) != nullptr;
+ Stm, Context)) != nullptr;
 }
 
 const Stmt *
@@ -125,7 +125,7 @@ ExprMutationAnalyzer::findDeclMutation(A
 
 const Stmt *ExprMutationAnalyzer::findDeclMutation(const Decl *Dec) {
   const auto Refs = match(
-  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), *Stm, *Context);
+  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), Stm, Context);
   for (const auto  : Refs) {
 const auto *E = RefNodes.getNodeAs("expr");
 if (findMutation(E))
@@

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341854: [clang-tidy] ExprMutationAnalyzer: construct from 
references. Fixes PR3 (authored by lebedevri, committed by ).

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -42,14 +42,14 @@
 bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   const auto *const E = selectFirst("expr", Results);
-  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+  return utils::ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
 SmallVector
 mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
-  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  utils::ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
 std::string buffer;
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -88,8 +88,8 @@
   // Because the fix (changing to `const auto &`) will introduce an unused
   // compiler warning which can't be suppressed.
   // Since this case is very rare, it is safe to ignore it.
-  if (!utils::ExprMutationAnalyzer(ForRange.getBody(), )
-  .isMutated() &&
+  if (!utils::ExprMutationAnalyzer(*ForRange.getBody(), Context)
+   .isMutated() &&
   !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
  Context)
.empty()) {
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -95,14 +95,14 @@
   // Do not trigger on non-const value parameters when they are mutated either
   // within the function body or within init expression(s) when the function is
   // a ctor.
-  if (utils::ExprMutationAnalyzer(Function->getBody(), Result.Context)
+  if (utils::ExprMutationAnalyzer(*Function->getBody(), *Result.Context)
   .isMutated(Param))
 return;
   // CXXCtorInitializer might also mutate Param but they're not part of function
   // body, so check them separately here.
   if (const auto *Ctor = dyn_cast(Function)) {
 for (const auto *Init : Ctor->inits()) {
-  if (utils::ExprMutationAnalyzer(Init->getInit(), Result.Context)
+  if (utils::ExprMutationAnalyzer(*Init->getInit(), *Result.Context)
   .isMutated(Param))
 return;
 }
Index: clang-tidy/utils/ExprMutationAnalyzer.h
===
--- clang-tidy/utils/ExprMutationAnalyzer.h
+++ clang-tidy/utils/ExprMutationAnalyzer.h
@@ -23,7 +23,7 @@
 /// a given statement.
 class ExprMutationAnalyzer {
 public:
-  ExprMutationAnalyzer(const Stmt *Stm, ASTContext *Context)
+  ExprMutationAnalyzer(const Stmt , ASTContext )
   : Stm(Stm), Context(Context) {}
 
   bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; }
@@ -44,8 +44,8 @@
   const Stmt *findRangeLoopMutation(const Expr *Exp);
   const Stmt *findReferenceMutation(const Expr *Exp);
 
-  const Stmt *const Stm;
-  ASTContext *const Context;
+  const Stmt 
+  ASTContext 
   llvm::DenseMap Results;
 };
 
Index: clang-tidy/utils/ExprMutationAnalyzer.cpp
===
--- clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -102,7 +102,7 @@
   hasDescendant(equalsNode(Exp,
   cxxNoexceptExpr())
  .bind("expr")),
- *Stm, *Context)) != nullptr;
+ Stm, Context)) != nullptr;
 }
 
 const Stmt *
@@ -125,7 +125,7 @@
 
 const Stmt *ExprMutationAnalyzer::findDeclMutation(const Decl *Dec) {
   const auto Refs = match(
-  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), *Stm, *Context);
+  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), Stm, Context);
   for (const auto  : Refs) {
 const auto *E = RefNodes.getNodeAs("expr");
 if

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

2018-09-10 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman updated this revision to Diff 164734.
arphaman marked 3 inline comments as done.
arphaman added a comment.

Remove dead code for filesystem update fileID matching.


Repository:
  rC Clang

https://reviews.llvm.org/D50926

Files:
  include/clang/Basic/SourceManager.h
  lib/Basic/SourceManager.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -425,6 +425,60 @@
   EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[10].Loc, Macros[11].Loc));
 }
 
+TEST_F(SourceManagerTest, findFileIDsForFile) {
+  const char *header = "int x;";
+
+  const char *main = "#include \n"
+ "#include \n";
+
+  std::unique_ptr HeaderBuf =
+  llvm::MemoryBuffer::getMemBuffer(header);
+  std::unique_ptr MainBuf =
+  llvm::MemoryBuffer::getMemBuffer(main);
+
+  const FileEntry *HeaderFile =
+  FileMgr.getVirtualFile("/test-header.h", HeaderBuf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(HeaderFile, std::move(HeaderBuf));
+  const FileEntry *MainFile =
+  FileMgr.getVirtualFile("main.cpp", MainBuf->getBufferSize(), 0);
+  SourceMgr.overrideFileContents(MainFile, std::move(MainBuf));
+  SourceMgr.setMainFileID(
+  SourceMgr.createFileID(MainFile, SourceLocation(), SrcMgr::C_User));
+
+  TrivialModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
+  HeaderSearch HeaderInfo(std::make_shared(), SourceMgr,
+  Diags, LangOpts, &*Target);
+  Preprocessor PP(std::make_shared(), Diags, LangOpts,
+  SourceMgr, PCMCache, HeaderInfo, ModLoader,
+  /*IILookup =*/nullptr,
+  /*OwnsHeaderSearch =*/false);
+  PP.Initialize(*Target);
+
+  PP.EnterMainSourceFile();
+
+  while (1) {
+Token tok;
+PP.Lex(tok);
+if (tok.is(tok::eof))
+  break;
+  }
+
+  llvm::SmallVector MainFileID =
+  SourceMgr.findFileIDsForFile(MainFile);
+  ASSERT_EQ(1U, MainFileID.size());
+  ASSERT_EQ(MainFileID[0], SourceMgr.getMainFileID());
+
+  llvm::SmallVector Files = SourceMgr.findFileIDsForFile(HeaderFile);
+
+  ASSERT_EQ(2U, Files.size());
+  ASSERT_NE(Files[0], Files[1]);
+  SourceLocation Loc1 = SourceMgr.translateLineCol(Files[0], 1, 1);
+  SourceLocation Loc2 = SourceMgr.translateLineCol(Files[1], 1, 1);
+  ASSERT_NE(Loc1, Loc2);
+  ASSERT_TRUE(SourceMgr.isBeforeInTranslationUnit(Loc1, Loc2));
+}
+
 #endif
 
 } // anonymous namespace
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1602,6 +1602,52 @@
   return translateLineCol(FirstFID, Line, Col);
 }
 
+bool SourceManager::findFileIDsForFile(
+const FileEntry *SourceFile,
+llvm::function_ref Callback) const {
+  assert(SourceFile && "Null source file!");
+
+  // Look through all of the local source locations.
+  for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
+bool Invalid = false;
+const SLocEntry  = getLocalSLocEntry(I, );
+if (Invalid)
+  return false;
+if (!SLoc.isFile())
+  continue;
+const ContentCache *FileContentCache = SLoc.getFile().getContentCache();
+if (!FileContentCache || !FileContentCache->OrigEntry)
+  continue;
+
+if (FileContentCache->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(I)))
+return true;
+}
+  }
+
+  // If that still didn't help, try the modules.
+  for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
+const SLocEntry  = getLoadedSLocEntry(I);
+if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
+SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(-int(I) - 2)))
+return true;
+}
+  }
+
+  return false;
+}
+
+llvm::SmallVector
+SourceManager::findFileIDsForFile(const FileEntry *SourceFile) const {
+  llvm::SmallVector Result;
+  findFileIDsForFile(SourceFile, [&](FileID F) {
+Result.push_back(F);
+return false;
+  });
+  return Result;
+}
+
 /// Get the FileID for the given file.
 ///
 /// If the source file is included multiple times, the FileID will be the
@@ -1650,72 +1696,17 @@
 }
   }
 
-  if (FirstFID.isInvalid()) {
-// The location we're looking for isn't in the main file; look
-// through all of the local source locations.
-for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
-  bool Invalid = false;
-  const SLocEntry  = getLocalSLocEntry(I, );
-  if (Invalid)
-return FileID();
-
-  if (SLoc.isFile() &&
-  SLoc.getFile().getContentCache() &&
-  SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
-FirstFID = FileID::get(I);
-break;
-  }
-}
-// If that still didn't help, try the modules.
-if (FirstFID.isInvalid()) {
-  for

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

2018-09-10 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1626
+if (FileContentCache->OrigEntry == SourceFile) {
+  if (Callback(FileID::get(I)))
+return true;

ioeric wrote:
> Should we check `FileID::get(I)` is valid?
That's not really necessary. The FileID we get should be valid as a local SLoc 
entry should have a corresponding FileID. The SLoc 0 is not a File loc entry so 
we can never get FileID::get(0) here.



Comment at: lib/Basic/SourceManager.cpp:1628
+return true;
+} else if (LookForFilesystemUpdates &&
+   (SourceFileName || (SourceFileName = llvm::sys::path::filename(

ioeric wrote:
> ioeric wrote:
> > The conditions here are pretty hard to follow and reason about. Could we 
> > maybe split them (some documentation would also help)?
> In the original version, file system updates are checked last (after 
> modules). Any reason to move it before modules? 
> 
> Also, it seems that this code path could also be run when `FileID`above is 
> invalid? So I wonder whether `else if` is correct here.
I just realized that the original file system code has never been executed and 
was just dead code! If we take a look at this logic:

```
for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
FileID IFileID;
IFileID.ID = I;
const SLocEntry  = getSLocEntry(IFileID, );
if (Invalid)
  return false;
```

You'll notice that the loop starts iterating at `0`. Get SLocEntry is called 
with FileID `0`, which sets the `Invalid` flag to true. Then we simply return, 
so the loop never reached the code below. Looks like it's a regression that 
happened years ago. I removed this code for now, but I'll reinstate it 
correctly in a follow-up patch.



Repository:
  rC Clang

https://reviews.llvm.org/D50926



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D51884#1229460, @shuaiwang wrote:

> LGTM :)


Thank you for the review!

In https://reviews.llvm.org/D51884#1229463, @JonasToth wrote:

> I feel that the `findMutation...` functions that take raw pointers should get 
> the assertions though, at least the ones in the public interface. Having them 
> for the private ones won't hurt either.


I could do that.
But should that come with tests?
Or do we want to also take references there?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

I am in favor of the change.

I feel that the `findMutation...` functions that take raw pointers should get 
the assertions though, at least the ones in the public interface. Having them 
for the private ones won't hurt either.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang accepted this revision.
shuaiwang added a comment.
This revision is now accepted and ready to land.

LGTM :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri updated this revision to Diff 164730.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Fixed diff.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -42,14 +42,14 @@
 bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   const auto *const E = selectFirst("expr", Results);
-  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+  return utils::ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
 SmallVector
 mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
-  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  utils::ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
 std::string buffer;
Index: clang-tidy/utils/ExprMutationAnalyzer.h
===
--- clang-tidy/utils/ExprMutationAnalyzer.h
+++ clang-tidy/utils/ExprMutationAnalyzer.h
@@ -23,7 +23,7 @@
 /// a given statement.
 class ExprMutationAnalyzer {
 public:
-  ExprMutationAnalyzer(const Stmt *Stm, ASTContext *Context)
+  ExprMutationAnalyzer(const Stmt , ASTContext )
   : Stm(Stm), Context(Context) {}
 
   bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; }
@@ -44,8 +44,8 @@
   const Stmt *findRangeLoopMutation(const Expr *Exp);
   const Stmt *findReferenceMutation(const Expr *Exp);
 
-  const Stmt *const Stm;
-  ASTContext *const Context;
+  const Stmt 
+  ASTContext 
   llvm::DenseMap Results;
 };
 
Index: clang-tidy/utils/ExprMutationAnalyzer.cpp
===
--- clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -102,7 +102,7 @@
   hasDescendant(equalsNode(Exp,
   cxxNoexceptExpr())
  .bind("expr")),
- *Stm, *Context)) != nullptr;
+ Stm, Context)) != nullptr;
 }
 
 const Stmt *
@@ -125,7 +125,7 @@
 
 const Stmt *ExprMutationAnalyzer::findDeclMutation(const Decl *Dec) {
   const auto Refs = match(
-  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), *Stm, *Context);
+  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), Stm, Context);
   for (const auto  : Refs) {
 const auto *E = RefNodes.getNodeAs("expr");
 if (findMutation(E))
@@ -200,7 +200,7 @@
AsNonConstRefArg, AsLambdaRefCaptureInit,
AsNonConstRefReturn))
 .bind("stmt")),
-*Stm, *Context);
+Stm, Context);
   return selectFirst("stmt", Matches);
 }
 
@@ -211,16 +211,16 @@
cxxDependentScopeMemberExpr(
hasObjectExpression(equalsNode(Exp)
 .bind("expr")),
-*Stm, *Context);
+Stm, Context);
   return findExprMutation(MemberExprs);
 }
 
 const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) {
   // Check whether any element of an array is mutated.
   const auto SubscriptExprs = match(
   findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp
   .bind("expr")),
-  *Stm, *Context);
+  Stm, Context);
   return findExprMutation(SubscriptExprs);
 }
 
@@ -233,7 +233,7 @@
implicitCastExpr(hasImplicitDestinationType(
nonConstReferenceType()
 .bind("expr")),
-*Stm, *Context);
+Stm, Context);
   return findExprMutation(Casts);
 }
 
@@ -245,7 +245,7 @@
 hasLoopVariable(
 varDecl(hasType(nonConstReferenceType())).bind("decl")),
 hasRangeInit(equalsNode(Exp,
-*Stm, *Context);
+Stm, Context);
   return findDeclMutation(LoopVars);
 }
 
@@ -265,7 +265,7 @@
   unless(hasParent(declStmt(hasParent(
   cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt"
   .bind("decl"))),
-  *Stm, *Context);
+  Stm, Context);
   return findDeclMutation(Refs);
 }
 
Index:

[PATCH] D51886: [analyzer][UninitializedObjectChecker] Using the new const methods of ImmutableList

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, 
whisperity.

Clang side changes to https://reviews.llvm.org/D51881. This is an improvement, 
and won't cause compilation errors if not commited together.


Repository:
  rC Clang

https://reviews.llvm.org/D51886

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp


Index: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -359,14 +359,6 @@
 //   Methods for FieldChainInfo.
 
//===--===//
 
-const FieldRegion *FieldChainInfo::getUninitRegion() const {
-  assert(!Chain.isEmpty() && "Empty fieldchain!");
-
-  // ImmutableList::getHead() isn't a const method, hence the not too nice
-  // implementation.
-  return (*Chain.begin()).getRegion();
-}
-
 bool FieldChainInfo::contains(const FieldRegion *FR) const {
   for (const FieldNode  : Chain) {
 if (Node.isSameRegion(FR))
@@ -380,7 +372,7 @@
 /// recursive function to print the fieldchain correctly. The last element in
 /// the chain is to be printed by `FieldChainInfo::print`.
 static void printTail(llvm::raw_ostream ,
-  const FieldChainInfo::FieldChainImpl *L);
+  const FieldChainInfo::FieldChain L);
 
 // FIXME: This function constructs an incorrect string in the following case:
 //
@@ -399,30 +391,29 @@
   if (Chain.isEmpty())
 return;
 
-  const FieldChainImpl *L = Chain.getInternalPointer();
-  const FieldNode  = L->getHead();
+  const FieldNode  = getHead();
 
   LastField.printNoteMsg(Out);
   Out << '\'';
 
   for (const FieldNode  : Chain)
 Node.printPrefix(Out);
 
   Out << "this->";
-  printTail(Out, L->getTail());
+  printTail(Out, Chain.getTail());
   LastField.printNode(Out);
   Out << '\'';
 }
 
 static void printTail(llvm::raw_ostream ,
-  const FieldChainInfo::FieldChainImpl *L) {
-  if (!L)
+  const FieldChainInfo::FieldChain L) {
+  if (L.isEmpty())
 return;
 
-  printTail(Out, L->getTail());
+  printTail(Out, L.getTail());
 
-  L->getHead().printNode(Out);
-  L->getHead().printSeparator(Out);
+  L.getHead().printNode(Out);
+  L.getHead().printSeparator(Out);
 }
 
 
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -107,7 +107,6 @@
 /// functions such as add() and replaceHead().
 class FieldChainInfo {
 public:
-  using FieldChainImpl = llvm::ImmutableListImpl;
   using FieldChain = llvm::ImmutableList;
 
 private:
@@ -134,8 +133,8 @@
   bool contains(const FieldRegion *FR) const;
   bool isEmpty() const { return Chain.isEmpty(); }
 
-  const FieldRegion *getUninitRegion() const;
-  const FieldNode () { return Chain.getHead(); }
+  const FieldNode () const { return Chain.getHead(); }
+  const FieldRegion *getUninitRegion() const { return getHead().getRegion(); }
 
   void printNoteMsg(llvm::raw_ostream ) const;
 };


Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -359,14 +359,6 @@
 //   Methods for FieldChainInfo.
 //===--===//
 
-const FieldRegion *FieldChainInfo::getUninitRegion() const {
-  assert(!Chain.isEmpty() && "Empty fieldchain!");
-
-  // ImmutableList::getHead() isn't a const method, hence the not too nice
-  // implementation.
-  return (*Chain.begin()).getRegion();
-}
-
 bool FieldChainInfo::contains(const FieldRegion *FR) const {
   for (const FieldNode  : Chain) {
 if (Node.isSameRegion(FR))
@@ -380,7 +372,7 @@
 /// recursive function to print the fieldchain correctly. The last element in
 /// the chain is to be printed by `FieldChainInfo::print`.
 static void printTail(llvm::raw_ostream ,
-  const FieldChainInfo::FieldChainImpl *L);
+  const FieldChainInfo::FieldChain L);
 
 // FIXME: This function constructs an incorrect string in the following case:
 //
@@ -399,30 +391,29

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:209-214
-  const auto MemberExprs =
-  
match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
-   cxxDependentScopeMemberExpr(
-   hasObjectExpression(equalsNode(Exp)
-.bind("expr")),
-*Stm, *Context);

Hm, looks like i failed to rebase the patch :)
Will fix momentarily.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884



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

[PATCH] D51884: [clang-tidy] ExprMutationAnalyzer: construct from references. Fixes PR38888

2018-09-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri created this revision.
lebedev.ri added reviewers: JonasToth, shuaiwang, alexfh.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: Szelethus, a.sidorin, xazax.hun.
Herald added a reviewer: george.karpenkov.

I have hit this the rough way, while trying to use this in 
https://reviews.llvm.org/D51870.

There is no particular point in storing the pointers, and moreover
the pointers are assumed to be non-null, and that assumption is not
enforced. If they are null, it won't be able to do anything good
with them anyway.

Initially i thought about simply adding asserts() that they are
not null, but taking/storing references looks like even cleaner solution?

Fixes PR3 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51884

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -42,14 +42,14 @@
 bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   const auto *const E = selectFirst("expr", Results);
-  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+  return utils::ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
 SmallVector
 mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
-  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  utils::ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
 std::string buffer;
Index: clang-tidy/utils/ExprMutationAnalyzer.h
===
--- clang-tidy/utils/ExprMutationAnalyzer.h
+++ clang-tidy/utils/ExprMutationAnalyzer.h
@@ -23,7 +23,7 @@
 /// a given statement.
 class ExprMutationAnalyzer {
 public:
-  ExprMutationAnalyzer(const Stmt *Stm, ASTContext *Context)
+  ExprMutationAnalyzer(const Stmt , ASTContext )
   : Stm(Stm), Context(Context) {}
 
   bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; }
@@ -44,8 +44,8 @@
   const Stmt *findRangeLoopMutation(const Expr *Exp);
   const Stmt *findReferenceMutation(const Expr *Exp);
 
-  const Stmt *const Stm;
-  ASTContext *const Context;
+  const Stmt 
+  ASTContext 
   llvm::DenseMap Results;
 };
 
Index: clang-tidy/utils/ExprMutationAnalyzer.cpp
===
--- clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -102,7 +102,7 @@
   hasDescendant(equalsNode(Exp,
   cxxNoexceptExpr())
  .bind("expr")),
- *Stm, *Context)) != nullptr;
+ Stm, Context)) != nullptr;
 }
 
 const Stmt *
@@ -125,7 +125,7 @@
 
 const Stmt *ExprMutationAnalyzer::findDeclMutation(const Decl *Dec) {
   const auto Refs = match(
-  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), *Stm, *Context);
+  findAll(declRefExpr(to(equalsNode(Dec))).bind("expr")), Stm, Context);
   for (const auto  : Refs) {
 const auto *E = RefNodes.getNodeAs("expr");
 if (findMutation(E))
@@ -200,27 +200,24 @@
AsNonConstRefArg, AsLambdaRefCaptureInit,
AsNonConstRefReturn))
 .bind("stmt")),
-*Stm, *Context);
+Stm, Context);
   return selectFirst("stmt", Matches);
 }
 
 const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
   // Check whether any member of 'Exp' is mutated.
-  const auto MemberExprs =
-  match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
-   cxxDependentScopeMemberExpr(
-   hasObjectExpression(equalsNode(Exp)
-.bind("expr")),
-*Stm, *Context);
+  const auto MemberExprs = match(
+  findAll(memberExpr(hasObjectExpression(equalsNode(Exp))).bind("expr")),
+  Stm, Context);
   return findExprMutation(MemberExprs);
 }
 
 const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) {
   // Check whether any element of an array is mutated.
   const auto SubscriptExprs = match(
   findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp
   .bind("expr")),
-  *Stm, *Context);
+  Stm, Context);
   return

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

I added more testcases for templates and improved the diagnostics with notes. 
This includes newly discovered false positives related to uninstantiated 
templates.

@alexfh, @hokein Would you like to see better diagnostics?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714



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

[PATCH] D51880: [ASTMatchers] add two matchers for dependent expressions

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth created this revision.
JonasToth added reviewers: aaron.ballman, alexfh, klimek.
Herald added a subscriber: cfe-commits.

The new matchers can be used to check if an expression is type- or 
value-dependent
in a templated context.
These matchers are used in a clang-tidy check and generally useful as the
problem of unresolved templates occurs more often in clang-tidy and they
provide an easy way to check for this issue.


Repository:
  rC Clang

https://reviews.llvm.org/D51880

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h


Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -774,6 +774,29 @@
   return InnerMatcher.matches(*Node.IgnoreParenCasts(), Finder, Builder);
 }
 
+/// Matches expressions that are type-dependant because the template type
+/// is not yet instantiated.
+///
+/// For example, the expressions "x" and "x + y" are type-dependent in
+/// the following code, but "y" is not type-dependent:
+/// \code
+///   template
+///   void add(T x, int y) {
+/// x + y;
+///   }
+/// \endcode
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }
+
+/// Matches expression that are value-dependant because they contain a
+/// non-type template parameter.
+///
+/// For example, the array bound of "Chars" in the following example is
+/// value-dependent.
+/// \code
+///   template struct meta_string;
+/// \endcode
+AST_MATCHER(Expr, isValueDependent) { return Node.isValueDependent(); }
+
 /// Matches expressions that match InnerMatcher after implicit casts and
 /// parentheses are stripped off.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2758,6 +2758,29 @@
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprisTypeDependent
+Matches expressions 
that are type-dependant because the template type
+is not yet instantiated.
+
+For example, the expressions "x" and "x + y" are type-dependent in
+the following code, but "y" is not type-dependent:
+  templatetypename T
+  void add(T x, int y) {
+x + y;
+  }
+
+
+
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprisValueDependent
+Matches expression 
that are value-dependant because they contain a
+non-type template parameter.
+
+For example, the array bound of "Chars" in the following example is
+value-dependent.
+  templateint Size, char (Chars)[Size] struct meta_string;
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html;>FieldDeclhasBitWidthunsigned Width
 Matches non-static data 
members that are bit-fields of the specified
 bit width.


Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -774,6 +774,29 @@
   return InnerMatcher.matches(*Node.IgnoreParenCasts(), Finder, Builder);
 }
 
+/// Matches expressions that are type-dependant because the template type
+/// is not yet instantiated.
+///
+/// For example, the expressions "x" and "x + y" are type-dependent in
+/// the following code, but "y" is not type-dependent:
+/// \code
+///   template
+///   void add(T x, int y) {
+/// x + y;
+///   }
+/// \endcode
+AST_MATCHER(Expr, isTypeDependent) { return Node.isTypeDependent(); }
+
+/// Matches expression that are value-dependant because they contain a
+/// non-type template parameter.
+///
+/// For example, the array bound of "Chars" in the following example is
+/// value-dependent.
+/// \code
+///   template struct meta_string;
+/// \endcode
+AST_MATCHER(Expr, isValueDependent) { return Node.isValueDependent(); }
+
 /// Matches expressions that match InnerMatcher after implicit casts and
 /// parentheses are stripped off.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2758,6 +2758,29 @@
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprisTypeDependent
+Matches expressions that are type-dependant because the template type
+is not yet instantiated.
+
+For example, the expressions "x" and "x + y" are type-dependent in
+the following code, but "y" is not type-dependent:
+  templatetypename T
+  void add(T x, int y) {
+x + y;
+  }
+
+
+
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprisValueDependent
+Matches expression that are value-dependant because they contain a
+non-type template parameter.
+
+For example, the array bound of "Chars" in the following example is
+value-dependent.
+  templateint Size, char (Chars)[Size] struct meta_string;
+
+
+

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth updated this revision to Diff 164721.
JonasToth added a comment.

- use the global ASTMatchers for dependent expressions


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -2,6 +2,7 @@
 
 namespace std {
 class exception {};
+class invalid_argument : public exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
@@ -36,38 +37,38 @@
   try {
 throw non_derived_exception();
 // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-NOTES: 10:1: note: type defined here
   } catch (non_derived_exception ) {
   }
   throw non_derived_exception();
   // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-  // CHECK-NOTES: 9:1: note: type defined here
+  // CHECK-NOTES: 10:1: note: type defined here
 
 // FIXME: More complicated kinds of inheritance should be checked later, but there is
 // currently no way use ASTMatchers for this kind of task.
 #if 0
   // Handle private inheritance cases correctly.
   try {
 throw bad_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 10:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 11:1: note: type defined here
 throw no_good_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 11:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 12:1: note: type defined here
 throw really_creative();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-// CHECK MESSAGES: 12:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+// CHECK NOTES: 13:1: note: type defined here
   } catch (...) {
   }
   throw bad_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 10:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 11:1: note: type defined here
   throw no_good_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 11:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 12:1: note: type defined here
   throw really_creative();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-  // CHECK MESSAGES: 12:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+  // CHECK NOTES: 13:1: note: type defined here
 #endif
 }
 
@@ -91,24 +92,40 @@
   throw deep_hierarchy(); // Ok
 
   try {
-throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+throw terrible_idea();  // Ok, but multiple inheritance isn't clean
   } catch (std::exception ) { // Can be caught as std::exception, even with multiple inheritance
   }
   throw terrible_idea(); // Ok, but multiple inheritance
 }
 
+void test_lambdas() {
+  auto BadLambda = []() { throw int(42); };
+  // CHECK-NOTES: [[@LINE-1]]:33: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  auto GoodLambda = []() { throw derived_exception(); };
+}
+
 // Templated function that throws exception based on template type
 template 
 void ThrowException() { throw T(); }
 // CHECK-NOTES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 120:1: note: type defined here
-// CHECK-NOTES: [[@LINE-3]]:31: warning: throwing an exception

[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.

2018-09-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
vsapsai added a comment.

Rebased on top of trunk and checked that this is still working. Please take a 
look.


https://reviews.llvm.org/D50539



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

[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a subscriber: aaron.ballman.
JonasToth added a comment.

>   Different from std::vector::operator[] which has two overloads for const 
> and non-const access, std::unique_ptr only has one const version of 
> `operator->`.
> 
> So for SmartPtr x; x->mf(); we only see a const operator being invoked on x. 
> mf is not a member of SmartPtr and the member call to mf is not on x 
> directly, we never followed that far.

I think the `operator->` is transitively called, isn't it? (see andrei 
alexandrescu talk here: https://youtu.be/ozOgzlxIsdg?t=15m55s where he gives a 
nice class for generic locking)
Maybe there is something more general at work? I think @aaron.ballman knows 
more about deeper language questions :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50883



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

[PATCH] D50953: [clang-tidy] Handle sugared reference types in ExprMutationAnalyzer

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

What happens to pointers in a typedef (in the sense of `*` instead of `&`)?




Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:51
 const auto nonConstReferenceType = [] {
-  return referenceType(pointee(unless(isConstQualified(;
+  return hasUnqualifiedDesugaredType(
+  referenceType(pointee(unless(isConstQualified();

Not directly related, but this matcher matches universal references, even 
though they might result in a value. (one of the bugs lebedevri reported).



Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:171
 
+  AST = tooling::buildASTFromCode("typedef int& IntRef;"
+  "void g(IntRef); void f() { int x; g(x); }");

i think the following tests should be added as well

- `typedef const int& IntRef;`
- `template  using TemplateRef = T&`

It would be better to have at least one positive and negative test (with 
`const` and without `const` in the typedef) for each of theses cases.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50953



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

[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus added a comment.

I ran the checker on LLVM/Clang and grpc, and saw no crashes at all! Quite 
confident about the current diff. However, I'm not 100% sure it doesn't break 
on Objective C++ codes. Can you recommend a project like that?


https://reviews.llvm.org/D51057



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

[PATCH] D51867: [Diagnostics] Add error handling to FormatDiagnostic()

2018-09-10 Thread Jan Korous via Phabricator via cfe-commits 
jkorous planned changes to this revision.
jkorous added a comment.

Hi Volodymyr, 
Thanks for the feedback - interesting points!

I see your point regarding NFC - I am going to drop it as you are right.

Two things about sanitizers come to mind:

1. You'd need to guarantee that you have all possible code paths (or at least 
those you are able to cover with asserts) covered in tests to be able to 
replace asserts with sanitizers. (I think that even if that would be feasible 
asserts would prove to be way simpler.)
2. I prefer explicit assert right in the place where an assumption is made to 
test somewhere else as it make understanding the code much easier.

> Those change loop iteration from "iterator != end" to "iterator < end". As it 
> is functionality change, I'd like to have tests to cover that.

Technically you are right but I assume (ok, busted) that without any bug in the 
iteration this is NFC. I will try to look if I can find some simple input that 
would break current implementation.

> Also I've fixed a few bugs with going past the end of buffer and bugs were 
> actually inside the loop, not with buffer range check. It is tempting to play 
> safe but it has a risk of hiding real bugs.

But that almost sounds as that we should write fragile code so bugs from other 
parts of codebase show up... Anyway, I will think about this a bit more, it's 
an interesting point.




Comment at: lib/Basic/Diagnostic.cpp:804
+
+  while (DiagStr < DiagEnd) {
 if (DiagStr[0] != '%') {

vsapsai wrote:
> For example, I wouldn't call this one NFC.
You are right, I am gonna drop the NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D51867



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

2018-09-10 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan updated this revision to Diff 164706.
leonardchan added a comment.

- Push and pop contexts for every parsed statement inside 
`ParseStatementOrDeclaration` instead of at the start and end of compound 
statements


Repository:
  rC Clang

https://reviews.llvm.org/D49511

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/TypePrinter.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaType.cpp
  test/Frontend/noderef.c
  test/Frontend/noderef_on_non_pointers.cpp
  test/Frontend/noderef_on_non_pointers.m
  test/Frontend/noderef_templates.cpp

Index: test/Frontend/noderef_templates.cpp
===
--- /dev/null
+++ test/Frontend/noderef_templates.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+template 
+int func(T NODEREF *a) { // expected-note 2 {{a declared here}}
+  return *a + 1; // expected-warning 2 {{dereferencing a; was declared with a 'noderef' type}}
+}
+
+void func() {
+  float NODEREF *f;
+  int NODEREF *i;
+  func(f); // expected-note{{in instantiation of function template specialization 'func' requested here}}
+  func(i); // expected-note{{in instantiation of function template specialization 'func' requested here}}
+}
Index: test/Frontend/noderef_on_non_pointers.m
===
--- /dev/null
+++ test/Frontend/noderef_on_non_pointers.m
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+@interface NSObject
++ (id)new;
+@end
+
+void func() {
+  id NODEREF obj = [NSObject new]; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
Index: test/Frontend/noderef_on_non_pointers.cpp
===
--- /dev/null
+++ test/Frontend/noderef_on_non_pointers.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -fblocks -verify %s
+
+/**
+ * Test 'noderef' attribute against other pointer-like types.
+ */
+
+#define NODEREF __attribute__((noderef))
+
+void Normal() {
+  int NODEREF i;// expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int NODEREF *i_ptr;   // ok
+  int NODEREF **i_ptr2; // ok
+  int *NODEREF i_ptr3;  // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int *NODEREF *i_ptr4; // ok
+
+  auto NODEREF *auto_i_ptr = i_ptr;
+  auto NODEREF auto_i = i; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+
+  struct {
+int x;
+int y;
+  } NODEREF *s;
+
+  int __attribute__((noderef(10))) * no_args; // expected-error{{'noderef' attribute takes no arguments}}
+}
+
+const int NODEREF *const_i_ptr;
+static int NODEREF *static_i_ptr;
+
+void ParenTypes() {
+  int NODEREF(*i_ptr);// ok (same as `int NODEREF *`)
+  int NODEREF *(*i_ptr2); // ok (same as `int NODEREF **`)
+}
+
+// Function declarations
+int NODEREF func();   // expected-warning{{'noderef' can only be used on an array or pointer type}}
+int NODEREF *func2(); // ok (returning pointer)
+
+typedef int NODEREF (*func3)(int); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+typedef int NODEREF *(*func4)(int);
+
+void Arrays() {
+  int NODEREF i_arr[10];  // ok
+  int NODEREF i_arr2[10][10]; // ok
+  int NODEREF *i_arr3[10];// ok
+  int NODEREF i_arr4[] = {1, 2};
+}
+
+void ParenArrays() {
+  int NODEREF(i_ptr[10]);
+  int NODEREF(i_ptr2[10])[10];
+}
+
+typedef int NODEREF *(*func5[10])(int);
+
+// Arguments
+void func6(int NODEREF x); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+void func7(int NODEREF *x);
+void func8() NODEREF;
+
+void References() {
+  int x = 2;
+  int NODEREF  = x; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+  int *xp = 
+  int NODEREF * = xp; // ok (reference to a NODEREF *)
+  int *NODEREF  = xp; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+void BlockPointers() {
+  typedef int NODEREF (^IntBlock)(); // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+class A {
+public:
+  int member;
+  int NODEREF *member2;
+  int NODEREF member3; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+};
+
+void MemberPointer() {
+  int NODEREF A::*var = ::member; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+}
+
+template 
+class B {
+  Ty NODEREF *member;
+  Ty NODEREF member2; // expected-warning{{'noderef' can only be used on an array or pointer type}}
+};
Index: test/Frontend/noderef.c
===
--- /dev/null
+++ test/Frontend/noderef.c
@@ -0,0 +1,206 @@
+// RUN:

[PATCH] D51650: Implement target_clones multiversioning

2018-09-10 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

Thanks for the feedback @rsmith!  I'm working through the lambda issue and a 
few other things, but will get to this as soon as I can.




Comment at: include/clang/Basic/Attr.td:2031-2042
+mutable unsigned ActiveArgIndex = 0;
+void AdvanceActiveArgIndex() const {
+  ++ActiveArgIndex;
+  while(ActiveArgIndex < featuresStrs_size()) {
+if (std::find(featuresStrs_begin(),
+  featuresStrs_begin() + ActiveArgIndex,
+  *(featuresStrs_begin() + ActiveArgIndex))

rsmith wrote:
> Sorry, I don't think it's acceptable from a design perspective to have 
> mutable state in an `Attr` object, even if you can ensure that only one 
> client of the `Attr` will be using the state at a time. CodeGen is going to 
> need to track its own index into the attribute's list of clones.
Alright, I'll see if I can figure that out.  I should probably do something 
similar for the cpu-dispatch, since it actually uses a very similar mechanism.



Comment at: include/clang/Basic/AttrDocs.td:1619-1621
+Note that unlike the ``target`` syntax, every option listed creates a new
+version, disregarding whether it is split on a comma inside or outside a 
string.
+The following will emit 4 versions of the function.

rsmith wrote:
> Can we warn on attributes that contain multiple strings where one or more 
> such strings contains a comma? That seems like it would always be user error.
> 
> I think I'd then prefer to document this as: "The versions can either be 
> listed as a comma-separated sequence of string literals or as a single string 
> literal containing a comma-separated list of versions. For compatibility with 
> GCC, the two formats can be mixed. For example, the following will emit 4 
> versions of the function:"
I don't see why not, the warning should be a fairly trivial addition.



Comment at: lib/CodeGen/CodeGenModule.cpp:961-970
   if (const auto *FD = dyn_cast(ND))
 if (FD->isMultiVersion() && !OmitMultiVersionMangling) {
   if (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
 AppendCPUSpecificCPUDispatchMangling(
 CGM, FD->getAttr(), Out);
+  else if (FD->isTargetClonesMultiVersion())
+AppendTargetClonesMangling(CGM, FD->getAttr(), Out);

rsmith wrote:
> This chain of `if`s and similar things elsewhere make me think we're missing 
> an abstraction. Perhaps `FunctionDecl` should have a `getMultiVersionAttr()` 
> and/or `getMultiVersionKind()` functions?
I'm not super sure what either buys us... The multiversion attributes are all 
somewhat different unfortunately, so they would need to be dispatched 
separately later.  The 'getMultiVersionKind' is perhaps useful, though its 
essentially what isXXXMultiVersion does.  I'll think on it, I agree that there 
is likely an abstraction somewhere between that can improve this...



Comment at: lib/CodeGen/CodeGenModule.cpp:1043
+
+std::pair SpecCanonicalGD{CanonicalGD, VersionID};
 

rsmith wrote:
> This target index should be part of the `GlobalDecl`, not tracked alongside 
> it, because it identifies which global we're talking about.
I see.  I can definitely do that, I was concerned that adding an int to the 
GlobalDecl would be an unacceptable increase in size.  



Comment at: lib/Sema/SemaDecl.cpp:9811-9812
 
   // Main isn't allowed to become a multiversion function, however it IS
   // permitted to have 'main' be marked with the 'target' optimization hint.
   if (NewFD->isMain()) {

rsmith wrote:
> Why can't `main` be multiversioned? That seems like an arbitrary restriction.
At the time of implementing 'target', I was unsure of (and still am) how to 
accomplish this.  It would seem that I'd need to make the entry point a wrapper 
that calls the ifunc.  GCC seems to improperly call this (it doesn't emit the 
'main' fucntion as far as I can tell).



Comment at: lib/Sema/SemaDeclAttr.cpp:3068-3071
+  if (!HasDefault) {
+S.Diag(AL.getLoc(), diag::err_target_clone_must_have_default);
+return;
+  }

rsmith wrote:
> Do we need any other validation here? What if there are duplicate versions?
GCC has this annoying feature of just IGNORING duplicate versions.  I likely 
could/should warn about this, but for GCC compat we probably want to support 
this.



Comment at: test/CodeGen/attr-cpuspecific.c:14-15
 
-__attribute__((cpu_specific(ivybridge)))
-void NotCalled(void){}
+__attribute__((cpu_specific(ivybridge))) inline void InlineSingleVersion(void) 
{}
+// CHECK: define available_externally void @InlineSingleVersion.S() 
#[[S:[0-9]+]]
+

rsmith wrote:
> Should this really have external linkage? These `.` suffixes are reserved for 
> vendor-specific manglings that typically should only be used for symbols

[PATCH] D51650: Implement target_clones multiversioning

2018-09-10 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: include/clang/Basic/Attr.td:2031-2042
+mutable unsigned ActiveArgIndex = 0;
+void AdvanceActiveArgIndex() const {
+  ++ActiveArgIndex;
+  while(ActiveArgIndex < featuresStrs_size()) {
+if (std::find(featuresStrs_begin(),
+  featuresStrs_begin() + ActiveArgIndex,
+  *(featuresStrs_begin() + ActiveArgIndex))

Sorry, I don't think it's acceptable from a design perspective to have mutable 
state in an `Attr` object, even if you can ensure that only one client of the 
`Attr` will be using the state at a time. CodeGen is going to need to track its 
own index into the attribute's list of clones.



Comment at: include/clang/Basic/AttrDocs.td:1619-1621
+Note that unlike the ``target`` syntax, every option listed creates a new
+version, disregarding whether it is split on a comma inside or outside a 
string.
+The following will emit 4 versions of the function.

Can we warn on attributes that contain multiple strings where one or more such 
strings contains a comma? That seems like it would always be user error.

I think I'd then prefer to document this as: "The versions can either be listed 
as a comma-separated sequence of string literals or as a single string literal 
containing a comma-separated list of versions. For compatibility with GCC, the 
two formats can be mixed. For example, the following will emit 4 versions of 
the function:"



Comment at: lib/CodeGen/CodeGenModule.cpp:961-970
   if (const auto *FD = dyn_cast(ND))
 if (FD->isMultiVersion() && !OmitMultiVersionMangling) {
   if (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
 AppendCPUSpecificCPUDispatchMangling(
 CGM, FD->getAttr(), Out);
+  else if (FD->isTargetClonesMultiVersion())
+AppendTargetClonesMangling(CGM, FD->getAttr(), Out);

This chain of `if`s and similar things elsewhere make me think we're missing an 
abstraction. Perhaps `FunctionDecl` should have a `getMultiVersionAttr()` 
and/or `getMultiVersionKind()` functions?



Comment at: lib/CodeGen/CodeGenModule.cpp:1043
+
+std::pair SpecCanonicalGD{CanonicalGD, VersionID};
 

This target index should be part of the `GlobalDecl`, not tracked alongside it, 
because it identifies which global we're talking about.



Comment at: lib/Sema/SemaDecl.cpp:9800-9807
   // Mixing Multiversioning types is prohibited.
-  if ((NewTA && NewCPUDisp) || (NewTA && NewCPUSpec) ||
-  (NewCPUDisp && NewCPUSpec)) {
+  if ((static_cast(NewTA) + static_cast(NewCPUDisp) +
+   static_cast(NewCPUSpec) + static_cast(NewTargetClones)) >
+  1) {
 S.Diag(NewFD->getLocation(), diag::err_multiversion_types_mixed);
 NewFD->setInvalidDecl();
 return true;

We should also disallow multiple `TargetClonesAttr`s on the same declaration. 
GCC allows this and then ignores all but the last such attribute; we can do 
better.



Comment at: lib/Sema/SemaDecl.cpp:9811-9812
 
   // Main isn't allowed to become a multiversion function, however it IS
   // permitted to have 'main' be marked with the 'target' optimization hint.
   if (NewFD->isMain()) {

Why can't `main` be multiversioned? That seems like an arbitrary restriction.



Comment at: lib/Sema/SemaDecl.cpp:9839
 
-  if (OldFD->isMultiVersion() && MVType == MultiVersioning::None) {
+  // MultiVersioned redeclarations aren't allowed to omit the attribute except
+  // for target_clones.

I would expect the plain English term to not have interior capitalization.



Comment at: lib/Sema/SemaDeclAttr.cpp:3011
 
+bool Sema::checkTargetClonesAttr(SourceLocation LiteralLoc, StringRef Str,
+ bool ,

Should be something like `checkTargetClonesAttrStr` to indicate that this is 
checking one specific string, not the entire attribute.



Comment at: lib/Sema/SemaDeclAttr.cpp:3018-3029
+  // Warn on empty at the beginning of a string.
+  if (Str.size() == 0 || Str[0] == ',')
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+   << Unsupported << None << "" << TargetClones;
+
+  while (Str.size() != 0) {
+// Remove the comma we found last time through.

When looping through comma-delimited strings, it's easier to use 
`StringRef::split`:

```
std::pair Parts = {{}, Str};
while (!Parts.second.empty()) {
  Parts = Parts.second.split(',');
  StringRef Cur = Parts.first.trim();
  // ...
}
```



Comment at: lib/Sema/SemaDeclAttr.cpp:3035-3043
+return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+   << Unsupported << Architecture
+   << Cur.drop_front(sizeof("arch=") - 1) << TargetClones;
+}

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

In https://reviews.llvm.org/D45679#1229176, @shuaiwang wrote:

> I have a rough idea about how `findPointeeMutation` would look like, I'm 
> pretty sure I can use a lot of your help :)
>  My current plan:
>
> - Finish the few existing pending changes
> - Move the analyzer to `clang/lib/Analysis` (likely remove 
> `PseudoConstantAnalysis` there as well since it appears to be dead code for 
> years)
> - Implement `findPointeeMutation`


Sounds like a solid plan. I think the 
`clang-tidy/utils/DeclRefExprUtils.{h,cpp}` are not necessary anymore, too. 
They are used in 2 clang-tidy checks. Maybe i can remove them as well :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina closed this revision.
kristina added a comment.

In https://reviews.llvm.org/D50246#1229191, @lebedev.ri wrote:

> In https://reviews.llvm.org/D50246#1229177, @kristina wrote:
>
> > In https://reviews.llvm.org/D50246#1229152, @rogfer01 wrote:
> >
> > > Hi @kristina .
> > >
> > > Sure, I didn't mean to do that broader change here. Apologies if it read 
> > > that way.
> > >
> > > Would it be acceptable to add an empty `--sysroot=` to the test? I can 
> > > post the change for review in another diff.
> > >
> > > Thanks a lot.
> >
> >
> > Yes, you can submit another diff for this specific test, just use "Update 
> > Diff" and add yours on top which should open it up for re-review or use 
> > "Commandeer Revision" (and then submit another diff since it was previously 
> > closed). Up to your judgement.
>
>
> Please don't perform necromancy on already committed and closed differentials 
> (unless the commit was reverted, of course).
>  Do open new differentials.


Fair enough, sorry I suggested that.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D50892: [analyzer][UninitializedObjectChecker] Correct dynamic type is acquired for record pointees

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:802
+struct DynTBase2 {
+  int x; // expected-note{{uninitialized field 'static_cast(this->bptr)->DynTBase2::x'}}
+};

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > Mmm, what's the value of casting to derived type and then specifying 
> > > > that we access the field of the base type anyway? Isn't `this->bptr->x` 
> > > > exactly what the user needs to know(?)
> > > True, but it's a one tough job to write `this->bptr->x` here and also a 
> > > correct note message for...
> > I guess don't try too hard, eg. say if it requires something of non-linear 
> > complexity it's probably not worth it (not because it'd be slow but because 
> > it'd be an indication that it might be not worth the effort).
> I actually invested some effort into this and I'm fairly certain I could pull 
> it off with O(n) complexity, but I'll probably just place a TODO in the code 
> for now, as I have other things I really want to get fixed first.
I think I'll leave this as is, it could be done, but it wouldn't be nice, and 
it wouldn't make anyone's life significantly easier. At least this report tells 
you the dynamic type of `this->bptr`, so I guess that's something.


https://reviews.llvm.org/D50892



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

[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341848: [clang-tidy] Handle unresolved expressions in 
ExprMutationAnalyzer (authored by shuaiwang, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50619?vs=160960=164704#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50619

Files:
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -114,6 +114,26 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
+TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
+  auto AST = tooling::buildASTFromCode(
+  "struct X { template  void mf(); };"
+  "template  void f() { X x; x.mf(); }");
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST =
+  tooling::buildASTFromCode("template  void f() { T x; x.mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCode(
+  "template  struct X;"
+  "template  void f() { X x; x.mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+}
+
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
   const auto AST = tooling::buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
@@ -292,6 +312,51 @@
   ElementsAre("std::forward(x)"));
 }
 
+TEST(ExprMutationAnalyzerTest, CallUnresolved) {
+  auto AST =
+  tooling::buildASTFromCode("template  void f() { T x; g(x); }");
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  void f() { char x[N]; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  void f(T t) { int x; g(t, x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(t, x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  void f(T t) { int x; t.mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("t.mf(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  struct S;"
+  "template  void f() { S s; int x; s.mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "struct S { template  void mf(); };"
+  "template  void f(S s) { int x; s.mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("s.mf(x)"));
+
+  AST = tooling::buildASTFromCode("template "
+  "void g(F f) { int x; f(x); } ");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("f(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "template  void f() { int x; (void)T(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
+}
+
 TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
   const auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
   const auto Results =
@@ -347,6 +412,21 @@
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
 }
 
+TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
+  const auto AST = tooling::buildASTFromCode(
+  "template  struct S { static constexpr int v = 8; };"
+  "template <> struct S { static constexpr int v = 4; };"
+  "void g(char*);"
+  "template  void f() { char x[S::v]; g(x); }"
+  "template <> void f() { char y[S::v]; g(y); }");
+  const auto ResultsX =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(ResultsX, AST.get()), ElementsAre("g(x)"));
+  const auto ResultsY =
+  match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
+}
+
 TEST(ExprMutationAnalyzerTest, FollowRefModified)

[clang-tools-extra] r341848 - [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

2018-09-10 Thread Shuai Wang via cfe-commits 
Author: shuaiwang
Date: Mon Sep 10 11:05:13 2018
New Revision: 341848

URL: http://llvm.org/viewvc/llvm-project?rev=341848=rev
Log:
[clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer

Summary:
- If a function is unresolved, assume it mutates its arguments
- Follow unresolved member expressions for nested mutations

Reviewers: aaron.ballman, JonasToth, george.karpenkov

Subscribers: xazax.hun, a.sidorin, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341848=341847=341848=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Mon Sep 
10 11:05:13 2018
@@ -145,11 +145,16 @@ const Stmt *ExprMutationAnalyzer::findDi
 hasUnaryOperand(equalsNode(Exp)));
 
   // Invoking non-const member function.
+  // A member function is assumed to be non-const when it is unresolved.
   const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
   const auto AsNonConstThis =
   expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), 
on(equalsNode(Exp))),
  cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp);
+ hasArgument(0, equalsNode(Exp))),
+ callExpr(callee(expr(anyOf(
+ 
unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
+ cxxDependentScopeMemberExpr(
+ hasObjectExpression(equalsNode(Exp);
 
   // Taking address of 'Exp'.
   // We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -165,10 +170,16 @@ const Stmt *ExprMutationAnalyzer::findDi
unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
 
   // Used as non-const-ref argument when calling a function.
+  // An argument is assumed to be non-const-ref when the function is 
unresolved.
   const auto NonConstRefParam = forEachArgumentWithParam(
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
-  const auto AsNonConstRefArg =
-  anyOf(callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam));
+  const auto AsNonConstRefArg = anyOf(
+  callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam),
+  callExpr(callee(expr(anyOf(unresolvedLookupExpr(), 
unresolvedMemberExpr(),
+ cxxDependentScopeMemberExpr(),
+ hasType(templateTypeParmType(),
+   hasAnyArgument(equalsNode(Exp))),
+  cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp;
 
   // Captured by a lambda by reference.
   // If we're initializing a capture with 'Exp' directly then we're 
initializing
@@ -195,9 +206,12 @@ const Stmt *ExprMutationAnalyzer::findDi
 
 const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
   // Check whether any member of 'Exp' is mutated.
-  const auto MemberExprs = match(
-  findAll(memberExpr(hasObjectExpression(equalsNode(Exp))).bind("expr")),
-  *Stm, *Context);
+  const auto MemberExprs =
+  
match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
+   cxxDependentScopeMemberExpr(
+   hasObjectExpression(equalsNode(Exp)
+.bind("expr")),
+*Stm, *Context);
   return findExprMutation(MemberExprs);
 }
 

Modified: 
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=341848=341847=341848=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp 
Mon Sep 10 11:05:13 2018
@@ -114,6 +114,26 @@ TEST(ExprMutationAnalyzerTest, NonConstM
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
+TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
+  auto AST = tooling::buildASTFromCode(
+  "struct X { template  void mf(); };"
+  "template  void f() { X x; x.mf(); }");
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST =
+  tooling::buildASTFromCode("template  void f() { T x; x.mf(); 
}");
+

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D50246#1229177, @kristina wrote:

> In https://reviews.llvm.org/D50246#1229152, @rogfer01 wrote:
>
> > Hi @kristina .
> >
> > Sure, I didn't mean to do that broader change here. Apologies if it read 
> > that way.
> >
> > Would it be acceptable to add an empty `--sysroot=` to the test? I can post 
> > the change for review in another diff.
> >
> > Thanks a lot.
>
>
> Yes, you can submit another diff for this specific test, just use "Update 
> Diff" and add yours on top which should open it up for re-review or use 
> "Commandeer Revision" (and then submit another diff since it was previously 
> closed). Up to your judgement.


Please don't perform necromancy on already committed and closed differentials 
(unless the commit was reverted, of course).
Do open new differentials.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a comment.

In https://reviews.llvm.org/D50246#1229152, @rogfer01 wrote:

> Hi @kristina .
>
> Sure, I didn't mean to do that broader change here. Apologies if it read that 
> way.
>
> Would it be acceptable to add an empty `--sysroot=` to the test? I can post 
> the change for review in another diff.
>
> Thanks a lot.


Yes, you can submit another diff for this specific test, just use "Update Diff" 
and add yours on top which should open it up for re-review or use "Commandeer 
Revision" (and then submit another diff since it was previously closed). Up to 
your judgement.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang added a comment.

I have a rough idea about how `findPointeeMutation` would look like, I'm pretty 
sure I can use a lot of your help :)
My current plan:

- Finish the few existing pending changes
- Move the analyzer to `clang/lib/Analysis` (likely remove 
`PseudoConstantAnalysis` there as well since it appears to be dead code for 
years)
- Implement `findPointeeMutation`

In https://reviews.llvm.org/D45679#1229071, @JonasToth wrote:

> @shuaiwang What are you working on next? Do you have an idea on how we
>  will handle the pointee of a pointer? That would be very interesting for
>  my const-correctness check. if you need help with anything, just say so.
>  I would like to support
>
> Am 10.09.2018 um 18:49 schrieb Shuai Wang via Phabricator:
>
> > shuaiwang added a comment.
> > 
> > In https://reviews.llvm.org/D45679#1183116, @george.karpenkov wrote:
> > 
> >> @aaron.ballman @alexfh @shuaiwang Would it be possible to move that code 
> >> into a matcher, or into a something which could be used from Clang? We 
> >> would also like to use similar functionality, but can't include stuff from 
> >> clang-tidy.
> > 
> > FYI I haven't forgot about this, I'll look into doing it after a few 
> > pending changes are in.
> > 
> > 
> >  Comment at: clang-tidy/utils/ExprMutationAnalyzer.h:38
> >  +  const Stmt *findDeclMutation(ArrayRef 
> > Matches);
> >  +  const Stmt *findDeclMutation(const Decl *Dec);
> >  +
> > 
> >  
> > 
> > lebedev.ri wrote:
> > 
> >> lebedev.ri wrote:
> >> 
> >>> lebedev.ri wrote:
> >>> 
>  @shuaiwang, @JonasToth hi.
>   Is it very intentional that this `findDeclMutation()` is private?
>  
>  Is there some other way i should be doing if i have a statement `S`,
>   a declRefExpr `D`, and i want to find the statement `Q`, which mutates
>   the underlying variable to which the declRefExpr `D` refers?
> >>> 
> >>> (the statement `Q` within the statement `S`, of course)
> >> 
> >> @shuaiwang after a disscussion about this in IRC with @JonasToth, i have 
> >> filed https://bugs.llvm.org/show_bug.cgi?id=3
> >>  But i'm failing to CC you there. Are you not registered in the bugzilla?
> > 
> > There's no real reason findDeclMutation is private other than that there 
> > wasn't a use case :)
> >  Feel free to make it public if you find it useful that way.
> > 
> > I'll take a look at the bug, thanks for reporting it!
> >  (I don't have an account there yet, I'm requesting one right now, will 
> > follow up in the bug)
> > 
> > Repository:
> > 
> >   rCTE Clang Tools Extra
> > 
> > https://reviews.llvm.org/D45679





Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341847: [clang-cl] Enable -march option (authored by aganea, 
committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51806

Files:
  include/clang/Driver/Options.td
  test/Driver/cl-options.c


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1931,7 +1931,7 @@
 def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, 
Group;
 def mwatchos_simulator_version_min_EQ : Joined<["-"], 
"mwatchos-simulator-version-min=">;
 def mwatchsimulator_version_min_EQ : Joined<["-"], 
"mwatchsimulator-version-min=">, Alias;
-def march_EQ : Joined<["-"], "march=">, Group;
+def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>;
 def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>;
 def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group;
 def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group;
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -610,6 +610,7 @@
 // RUN: -flto \
 // RUN: -fmerge-all-constants \
 // RUN: -no-canonical-prefixes \
+// RUN: -march=skylake \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1931,7 +1931,7 @@
 def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group;
 def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">;
 def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias;
-def march_EQ : Joined<["-"], "march=">, Group;
+def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>;
 def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>;
 def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group;
 def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group;
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -610,6 +610,7 @@
 // RUN: -flto \
 // RUN: -fmerge-all-constants \
 // RUN: -no-canonical-prefixes \
+// RUN: -march=skylake \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r341847 - [clang-cl] Enable -march option

2018-09-10 Thread Alexandre Ganea via cfe-commits 
Author: aganea
Date: Mon Sep 10 10:54:32 2018
New Revision: 341847

URL: http://llvm.org/viewvc/llvm-project?rev=341847=rev
Log:
[clang-cl] Enable -march option

This change allows usage of -march when using the clang-cl driver. This is 
similar to MSVC's /arch; however -march can target precisely all supported 
CPUs, while /arch has a more restricted set.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=341847=341846=341847=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Sep 10 10:54:32 2018
@@ -1931,7 +1931,7 @@ def mappletvsimulator_version_min_EQ : J
 def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, 
Group;
 def mwatchos_simulator_version_min_EQ : Joined<["-"], 
"mwatchos-simulator-version-min=">;
 def mwatchsimulator_version_min_EQ : Joined<["-"], 
"mwatchsimulator-version-min=">, Alias;
-def march_EQ : Joined<["-"], "march=">, Group;
+def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>;
 def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>;
 def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group;
 def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=341847=341846=341847=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Mon Sep 10 10:54:32 2018
@@ -610,6 +610,7 @@
 // RUN: -flto \
 // RUN: -fmerge-all-constants \
 // RUN: -no-canonical-prefixes \
+// RUN: -march=skylake \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 


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

RE: [PATCH] D51806: [clang-cl] Enable -march

2018-09-10 Thread Alexandre Ganea via cfe-commits 
Yes, indeed. The code in clang/lib/Driver/ToolChains/Arch/X86.cpp makes that 
–march is always parsed, leaving out /arch unused, no matter in which order 
they appear.

De : Nico Weber 
Envoyé : 10 septembre 2018 12:14
À : reviews+d51806+public+c25b17a1aa94d...@reviews.llvm.org; Senthil Kumar 
Selvaraj via Phabricator 
Cc : Alexandre Ganea ; cfe-commits 

Objet : Re: [PATCH] D51806: [clang-cl] Enable -march

On Mon, Sep 10, 2018 at 10:33 AM Hans Wennborg via Phabricator via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
hans added a comment.

In https://reviews.llvm.org/D51806#1228893, @aganea wrote:

> @hans Just an after thought: maybe we should prevent usage of `-march=` and 
> `/arch:` at the same time. What do you think? I can add another patch for 
> that purpose.


Hmm, yes, at least we should warn or do something smart. Currently it doesn't 
look like they'd interact well together in x86::getX86TargetCPU

Wouldn't you get an "unused arg" for /arch if you use -march and /arch at the 
same time?



Repository:
  rC Clang

https://reviews.llvm.org/D51806



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

[PATCH] D51867: [Diagnostics][NFC] Add error handling to FormatDiagnostic()

2018-09-10 Thread Volodymyr Sapsai via Phabricator via cfe-commits 
vsapsai added a comment.

Regarding the asserts to catch potential problems, seems most of them are for 
buffer overflows. Aren't sanitizers catching those cases, specifically Address 
Sanitizer? I haven't checked, just seems it would be good to check buffer 
overflow automatically instead of using explicit asserts.

Also there are a few changes I wouldn't call NFC. Those change loop iteration 
from "iterator != end" to "iterator < end". As it is functionality change, I'd 
like to have tests to cover that. Also I've fixed a few bugs with going past 
the end of buffer and bugs were actually inside the loop, not with buffer range 
check. It is tempting to play safe but it has a risk of hiding real bugs.




Comment at: lib/Basic/Diagnostic.cpp:804
+
+  while (DiagStr < DiagEnd) {
 if (DiagStr[0] != '%') {

For example, I wouldn't call this one NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D51867



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added a comment.

Hi @kristina .

Sure, I didn't mean to do that broader change here. Apologies if it read that 
way.

Would it be acceptable to add an empty `--sysroot=` to the test? I can post the 
change for review in another diff.

Thanks a lot.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D51650: Implement target_clones multiversioning

2018-09-10 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

Based on feedback from @rsmith I've been trying to get this to work with 
Lambdas like GCC does, though I'm quite confused about how to go about it here. 
 It seems that the lambda definition call operator is deferred, however I'm 
having a hard time making sure that this happens in the "IFunc" case. I'm still 
working on it, but it might be a bit.


https://reviews.llvm.org/D51650



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

[PATCH] D51341: [HEADER] Overloadable function candidates for half/double types

2018-09-10 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:6063
+  // OpenCL: A candidate function that uses extentions that are not enabled or
+  // supported is not viable.
+  if (getLangOpts().OpenCL) {

sidorovd wrote:
> Anastasia wrote:
> > I would imagine if extension isn't supported the candidate function with 
> > data type defined by extension shouldn't be available at all during 
> > compilation?
> > 
> > Also is there any good way to generalize this for all types and extensions 
> > including vendor ones that are added with the pragmas?
> > https://clang.llvm.org/docs/UsersManual.html#opencl-extensions
> > I would imagine if extension isn't supported the candidate function with 
> > data type defined by extension shouldn't be available at all during 
> > compilation?
> 
> Yes, you are right.
> 
> > Also is there any good way to generalize this for all types and extensions 
> > including vendor ones that are added with the pragmas?
> https://clang.llvm.org/docs/UsersManual.html#opencl-extensions
> 
> There might be a way but I can't properly answer this question right now. By 
> default types and extensions aren't associated to each other.
We have a map of types and associated extensions with them in 
`OpenCLTypeExtMap` in Sema.h... not sure what would be the cost of such generic 
check though.



Comment at: test/SemaOpenCL/half-double-overload.cl:18
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);

sidorovd wrote:
> Anastasia wrote:
> > Wondering if better message could be:
> >   candidate unavailable as it requires OpenCL extension to be disabled
> > 
> > Could it also print the extension name?
> Sounds good. And I'd prefer to split it into another patch, because it would 
> affect unrelated to the change tests and also require changes in Sema to 
> allow extension name printing. 
Ok, makes sense for this to be a separate change!


https://reviews.llvm.org/D51341



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

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

2018-09-10 Thread Mike Rice via Phabricator via cfe-commits 
mikerice updated this revision to Diff 164698.
mikerice marked an inline comment as done.
mikerice added a comment.

Updated to use two options: -pch-through-header-create and 
-pch-through-header-use.


https://reviews.llvm.org/D51391

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/Pragma.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/ParseAST.cpp
  test/Driver/cl-pch.cpp
  test/PCH/Inputs/pch-hdrstop-use.cpp
  test/PCH/Inputs/pch-no-hdrstop-use.cpp
  test/PCH/pch-hdrstop-err.cpp
  test/PCH/pch-hdrstop-warn.cpp
  test/PCH/pch-hdrstop.cpp
  test/PCH/pch-no-hdrstop.cpp

Index: lib/Parse/ParseAST.cpp
===
--- lib/Parse/ParseAST.cpp
+++ lib/Parse/ParseAST.cpp
@@ -141,26 +141,26 @@
 CleanupParser(ParseOP.get());
 
   S.getPreprocessor().EnterMainSourceFile();
-  if (!S.getPreprocessor().getCurrentLexer()) {
-// If a PCH through header is specified that does not have an include in
-// the source, there won't be any tokens or a Lexer.
-return;
-  }
-
-  P.Initialize();
-
-  Parser::DeclGroupPtrTy ADecl;
   ExternalASTSource *External = S.getASTContext().getExternalSource();
   if (External)
 External->StartTranslationUnit(Consumer);
 
-  for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
-   AtEOF = P.ParseTopLevelDecl(ADecl)) {
-// If we got a null return and something *was* parsed, ignore it.  This
-// is due to a top-level semicolon, an action override, or a parse error
-// skipping something.
-if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
-  return;
+  // If a PCH through header is specified that does not have an include in
+  // the source, or a PCH is being created with #pragma hdrstop with nothing
+  // after the pragma, there won't be any tokens or a Lexer.
+  bool HaveLexer = S.getPreprocessor().getCurrentLexer();
+
+  if (HaveLexer) {
+P.Initialize();
+Parser::DeclGroupPtrTy ADecl;
+for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
+ AtEOF = P.ParseTopLevelDecl(ADecl)) {
+  // If we got a null return and something *was* parsed, ignore it.  This
+  // is due to a top-level semicolon, an action override, or a parse error
+  // skipping something.
+  if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
+return;
+}
   }
 
   // Process any TopLevelDecls generated by #pragma weak.
@@ -179,7 +179,7 @@
   std::swap(OldCollectStats, S.CollectStats);
   if (PrintStats) {
 llvm::errs() << "\nSTATISTICS:\n";
-P.getActions().PrintStats();
+if (HaveLexer) P.getActions().PrintStats();
 S.getASTContext().PrintStats();
 Decl::PrintStats();
 Stmt::PrintStats();
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2982,22 +2982,9 @@
 }
   }
 
-  // Diagnose unsupported forms of /Yc /Yu. Ignore /Yc/Yu for now if:
-  // * no filename after it
-  // * both /Yc and /Yu passed but with different filenames
-  // * corresponding file not also passed as /FI
+  // Ignore /Yc/Yu if both /Yc and /Yu passed but with different filenames.
   Arg *YcArg = Args.getLastArg(options::OPT__SLASH_Yc);
   Arg *YuArg = Args.getLastArg(options::OPT__SLASH_Yu);
-  if (YcArg && YcArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YcArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yc);
-YcArg = nullptr;
-  }
-  if (YuArg && YuArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YuArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yu);
-YuArg = nullptr;
-  }
   if (YcArg && YuArg && strcmp(YcArg->getValue(), YuArg->getValue()) != 0) {
 Diag(clang::diag::warn_drv_ycyu_different_arg_clang_cl);
 Args.eraseArg(options::OPT__SLASH_Yc);
@@ -4279,11 +4266,11 @@
 // extension of .pch is assumed. "
 if (!llvm::sys::path::has_extension(Output))
   Output += ".pch";
-  } else if (Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc)) {
-Output = YcArg->getValue();
-llvm::sys::path::replace_extension(Output, ".pch");
   } else {
-Output = BaseName;
+if (Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc))
+  Output = YcArg->getValue();
+if (Output.empty())
+  Output = BaseName;
 llvm::sys::path::replace_extension(Output, ".pch");
   }
   return Output.str();
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1105,10 +1105,19 @@
   StringRef ThroughHeader = YcArg ?

[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus added a comment.

There are some nasty environment issues I'm dealing with, so the issue is the 
lack of a well functioning creduce, but I'll get it done eventually.


https://reviews.llvm.org/D51300



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

[PATCH] D51680: [analyzer][UninitializedObjectChecker] New flag to ignore records based on it's fields

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:468
+static bool isUnionLike(const RecordDecl *RD) {
+  llvm::Regex ContainsKindOrTag("[kK]ind|[tT]ag");
+

george.karpenkov wrote:
> 1. Since you are using `match`, you would reject an extra prefix - but what 
> about an extra suffix?
> 2. Would it make sense to expose the entire regexp as a flag? Then you could 
> remove the boolean flag (if you can change the regexp to match if only the 
> entire field name matches, then an empty regexp would correspond to "no 
> matches")
1. Added a new test case, seems to be fine ^-^
2. Sounds cool, so I made it happen.


https://reviews.llvm.org/D51680



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

[PATCH] D51680: [analyzer][UninitializedObjectChecker] New flag to ignore records based on it's fields

2018-09-10 Thread Umann Kristóf via Phabricator via cfe-commits 
Szelethus updated this revision to Diff 164696.
Szelethus retitled this revision from "[analyzer][UninitializedObjectChecker] 
New flag to ignore union-like constructs" to 
"[analyzer][UninitializedObjectChecker] New flag to ignore records based on 
it's fields".
Szelethus edited the summary of this revision.
Szelethus added a comment.

Generalized the patch so that the user can supply the pattern that is matched 
against the fields of the records.


https://reviews.llvm.org/D51680

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -356,6 +356,13 @@
 whether the object itself is initialized. Defaults to false. 
 -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true.
   
+  
+"IgnoreRecordsWithField" (string). If supplied, the checker will not
+ analyze structures that have a field with a name or type name that
+ matches the given pattern. Defaults to "".
+
+ -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind".
+  
 
 
 
Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
===
--- /dev/null
+++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
@@ -0,0 +1,136 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \
+// RUN:   -std=c++11 -verify  %s
+
+// expected-no-diagnostics
+
+// Both type and name contains "kind".
+struct UnionLikeStruct1 {
+  enum Kind {
+volume,
+area
+  } kind;
+
+  int Volume;
+  int Area;
+
+  UnionLikeStruct1(Kind kind, int Val) : kind(kind) {
+switch (kind) {
+case volume:
+  Volume = Val;
+  break;
+case area:
+  Area = Val;
+  break;
+}
+  }
+};
+
+void fUnionLikeStruct1() {
+  UnionLikeStruct1 t(UnionLikeStruct1::volume, 10);
+}
+
+// Only name contains "kind".
+struct UnionLikeStruct2 {
+  enum Type {
+volume,
+area
+  } kind;
+
+  int Volume;
+  int Area;
+
+  UnionLikeStruct2(Type kind, int Val) : kind(kind) {
+switch (kind) {
+case volume:
+  Volume = Val;
+  break;
+case area:
+  Area = Val;
+  break;
+}
+  }
+};
+
+void fUnionLikeStruct2() {
+  UnionLikeStruct2 t(UnionLikeStruct2::volume, 10);
+}
+
+// Only type contains "kind".
+struct UnionLikeStruct3 {
+  enum Kind {
+volume,
+area
+  } type;
+
+  int Volume;
+  int Area;
+
+  UnionLikeStruct3(Kind type, int Val) : type(type) {
+switch (type) {
+case volume:
+  Volume = Val;
+  break;
+case area:
+  Area = Val;
+  break;
+}
+  }
+};
+
+void fUnionLikeStruct3() {
+  UnionLikeStruct3 t(UnionLikeStruct3::volume, 10);
+}
+
+// Only type contains "tag".
+struct UnionLikeStruct4 {
+  enum Tag {
+volume,
+area
+  } type;
+
+  int Volume;
+  int Area;
+
+  UnionLikeStruct4(Tag type, int Val) : type(type) {
+switch (type) {
+case volume:
+  Volume = Val;
+  break;
+case area:
+  Area = Val;
+  break;
+}
+  }
+};
+
+void fUnionLikeStruct4() {
+  UnionLikeStruct4 t(UnionLikeStruct4::volume, 10);
+}
+
+// Both name and type name contains but does not equal to tag/kind.
+struct UnionLikeStruct5 {
+  enum WhateverTagBlahBlah {
+volume,
+area
+  } FunnyKindName;
+
+  int Volume;
+  int Area;
+
+  UnionLikeStruct5(WhateverTagBlahBlah type, int Val) : FunnyKindName(type) {
+switch (FunnyKindName) {
+case volume:
+  Volume = Val;
+  break;
+case area:
+  Area = Val;
+  break;
+}
+  }
+};
+
+void fUnionLikeStruct5() {
+  UnionLikeStruct5 t(UnionLikeStruct5::volume, 10);
+}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -107,6 +107,10 @@
 static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
   CheckerContext );
 
+/// Checks whether RD contains a field with a name or type name that matches
+/// \p Pattern.
+static bool shouldIgnoreRecord(const RecordDecl *RD, StringRef Pattern);
+
 //===--===//
 //  Methods for UninitializedObjectChecker.

[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.

2018-09-10 Thread Stella Stamenova via Phabricator via cfe-commits 
stella.stamenova added a comment.

I saw this test fail in our testing as well. I think the matches are too broad 
and I suspect this will fail intermittently on the bots too causing confusion:

  Command Output (stderr):
  --
  
/vstsdrive/_work/10/s/llvm/tools/clang/test/Driver/print-multi-directory.c:19:32:
 error: CHECK-X86_64-MULTILIBS-NOT: excluded string found in input
  // CHECK-X86_64-MULTILIBS-NOT: 32
 ^
  :1:199: note: found here
  clang version 8.0.0 
(https://yn744hjyhitxavgbejjp2xsdymxivo4b6r7yrbigmz6v3uodg...@edgetools.visualstudio.com/llvm/_git/clang
 5df831d3873a0affc7c7e1d908978f68222737f1) (llvm/_git/llvm 
3e62673386d63e53205e2b6b92955c5506225e38)


^~


Repository:
  rC Clang

https://reviews.llvm.org/D51354



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

[PATCH] D51416: [RTTI] Align rtti types to prevent over-alignment

2018-09-10 Thread Dave Green via Phabricator via cfe-commits 
dmgreen updated this revision to Diff 164693.
dmgreen added a comment.

Now using getABITypeAlignment, plus added a simple test


https://reviews.llvm.org/D51416

Files:
  lib/CodeGen/CGVTT.cpp
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGenCXX/microsoft-abi-vbtables.cpp
  test/CodeGenCXX/vtable-align.cpp
  test/CodeGenCXX/vtable-linkage.cpp

Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -98,10 +98,10 @@
 
 // C has no key function, so its vtable should have weak_odr linkage
 // and hidden visibility (rdar://problem/7523229).
-// CHECK-DAG: @_ZTV1C = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1C = linkonce_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTT1C = linkonce_odr unnamed_addr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTV1C = linkonce_odr unnamed_addr constant {{.*}}, comdat, align 8{{$}}
+// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1C = linkonce_odr constant {{.*}}, comdat, align 8{{$}}
+// CHECK-DAG: @_ZTT1C = linkonce_odr unnamed_addr constant {{.*}}, comdat, align 8{{$}}
 
 // D has a key function that is defined in this translation unit so its vtable is
 // defined in the translation unit.
@@ -120,27 +120,27 @@
 // defined in this translation unit, so its vtable should have
 // weak_odr linkage.
 // CHECK-DAG: @_ZTV1EIsE = weak_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1EIsE = weak_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1EIsE = weak_odr constant {{.*}}, comdat, align 8{{$}}
 
 // F is an explicit template instantiation without a key
 // function, so its vtable should have weak_odr linkage
 // CHECK-DAG: @_ZTV1FIsE = weak_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1FIsE = weak_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1FIsE = weak_odr constant {{.*}}, comdat, align 8{{$}}
 
 // E is an implicit template instantiation with a key function
 // defined in this translation unit, so its vtable should have
 // linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1EIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1EIlE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1EIlE = linkonce_odr constant {{.*}}, comdat, align 8{{$}}
 
 // F is an implicit template instantiation with no key function,
 // so its vtable should have linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1FIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1FIlE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1FIlE = linkonce_odr constant {{.*}}, comdat, align 8{{$}}
 
 // F is an explicit template instantiation declaration without a
 // key function, so its vtable should have external linkage.
@@ -171,8 +171,8 @@
 // F is an explicit specialization without a key function, so
 // its vtable should have linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1FIcE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat{{$}}
-// CHECK-DAG: @_ZTI1FIcE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
+// CHECK-DAG: @_ZTI1FIcE = linkonce_odr constant {{.*}}, comdat, align 8{{$}}
 
 // CHECK-DAG: @_ZTV1GIiE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
 template 
Index: test/CodeGenCXX/vtable-align.cpp
===
--- test/CodeGenCXX/vtable-align.cpp
+++ test/CodeGenCXX/vtable-align.cpp
@@ -10,5 +10,8 @@
 void A::f() {}
 
 // CHECK-32: @_ZTV1A = unnamed_addr constant { [5 x i8*] } { [5 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1hEv to i8*)] }, align 4
-
+// CHECK-32: @_ZTS1A = constant [3 x i8] c"1A\00", align 1
+// CHECK-32: @_ZTI1A = constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i32 2) to i8*), i8* getelementptr inbounds

[PATCH] D48581: [AArch64] Support reserving x1-7 registers.

2018-09-10 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

Thanks for the other half of this feature!




Comment at: test/Driver/aarch64-fixed-x-register.c:35
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X20 < %t %s
+// CHECK-FIXED-X20: "-target-feature" "+reserve-x20"

Is it worth checking a combination of these flags together work as expected 
(since I think you added tests that do that to llvm)?


https://reviews.llvm.org/D48581



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

[PATCH] D51544: [OpenCL] Split opencl-c.h header

2018-09-10 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia added a comment.

> With this setup, we can compile opencl-c-common.h, opencl-c-fp16.h and
>  opencl-c-fp64.h into PCHs with one set of extensions/OpenCL version,
>  and use them for any other set of extensions/OpenCL version. Clang
>  will detect this and throw out an error, which can be safely disabled
>  by -fno-validate-pch option.

However, keeping this as a permanent solution is unsafe. Because this way can 
result in unexpected errors to be silent out and allow erroneous configurations 
to be accepted successfully without any notification. So I am wondering if 
there is any plan to put a proper solution in place at some point?


Repository:
  rC Clang

https://reviews.llvm.org/D51544



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Kristina Brooks via Phabricator via cfe-commits 
kristina added a comment.

In https://reviews.llvm.org/D50246#1229068, @rogfer01 wrote:

> Thanks for reopening this @kristina.
>
> I suggest passing `--sysroot=` to make sure we see the expected behaviour 
> when the sysroot is actually empty.
>
> Note that this would not really test the scenario where `DEFAULT_SYSROOT` is 
> empty **and** no `--sysroot` appears in the command line. I'm not sure if we 
> really want to test that case (but if we do, I think we will have to move 
> that case into a test of its own and add a //feature// in `lit.cfg.py` that 
> describes that clang does not have any built-in default sysroot).
>
> Thoughts?


Seems like a fairly niche case, you can request changes to this revision but 
there's been no regressions otherwise, buildbots have been handling this fine.

I think this may be something that would belong in a separate diff for the test 
suite, not for this particular diff. You can always just submit a diff for the 
test suite if you think it should have that option and if it would be useful as 
a general thing.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D51864: [Tooling] Restore working dir in ClangTool.

2018-09-10 Thread Eric Liu via Phabricator via cfe-commits 
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rC Clang

https://reviews.llvm.org/D51864



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

[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-10 Thread Hamza Sood via Phabricator via cfe-commits 
hamzasood created this revision.
hamzasood added reviewers: rnk, EricWF, compnerd, smeenai.
Herald added subscribers: cfe-commits, ldionne, mgorny.

The patch fixes a few issues that arise when using libc++ on Windows.
Specifically:

1. The CMake configuration added `-Wall` to the compilation, which actually 
means `-Weverything` in MSVC. This led to several thousand warnings per file.
2. The experimental library was enabled by default. It doesn't really work 
(yet) on Windows, and in fact the build docs suggest to disable it, so I don't 
think that was a sensible default.
3. There were some other errors that caused compilation errors in some of the 
tests.

I've identified a few other issues, but I'm not sure how best to fix them:

1. `support/win32/locale_win32.h` includes `xlocinfo.h`, which ends up 
including `yvals_core.h`. That MSVC header defines feature test macros, among 
other things, that clash with macros defined in libc++. This is causing lots of 
test errors.
2. `` includes the ucrt header `new.h` in the hope that it'll declare 
`get_new_handler` etc. but `new.h` really just goes back and includes ``. 
The end result is that nothing actually gets declared and so we're missing a 
few declarations, which causes lots of compilation errors.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868

Files:
  CMakeLists.txt
  include/filesystem
  test/std/strings/c.strings/cuchar.pass.cpp
  test/support/test_macros.h
  test/support/verbose_assert.h

Index: test/support/verbose_assert.h
===
--- test/support/verbose_assert.h
+++ test/support/verbose_assert.h
@@ -21,18 +21,18 @@
 
 template ::value ? 1
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::cerr << val; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::wcerr << val; }
 };
 
@@ -86,14 +86,14 @@
 template 
 friend LogType& operator<<(LogType& log, Tp const& value) {
   if (!log.is_disabled) {
-SelectStream::Print(value);
+SelectErrStream::Print(value);
   }
   return log;
 }
 
 friend LogType& operator<<(LogType& log, EndLType* m) {
   if (!log.is_disabled) {
-SelectStream::Print(m);
+std::cerr << m;
   }
   return log;
 }
Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -143,11 +143,6 @@
 #  define TEST_HAS_C11_FEATURES
 #  define TEST_HAS_TIMESPEC_GET
 #endif
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library
-#  define TEST_HAS_TIMESPEC_GET
-#endif
 #  endif
 #endif
 
Index: test/std/strings/c.strings/cuchar.pass.cpp
===
--- test/std/strings/c.strings/cuchar.pass.cpp
+++ test/std/strings/c.strings/cuchar.pass.cpp
@@ -8,6 +8,9 @@
 //===--===//
 //
 // XFAIL: *
+//
+// The MSVC stdlib has cuchar, which causes this test to pass unexpectedly.
+// UNSUPPORTED: windows
 
 // 
 
Index: include/filesystem
===
--- include/filesystem
+++ include/filesystem
@@ -1393,7 +1393,6 @@
 return __storage_->__what_.c_str();
   }
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);
 
 private:
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -67,7 +67,7 @@
 option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
-option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
+option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" $)
 set(ENABLE_FILESYSTEM_DEFAULT ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 if (WIN32)
   set(ENABLE_FILESYSTEM_DEFAULT OFF)
@@ -542,8 +542,13 @@
 
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+if (MSVC)
+add_compile_flags(/W4)
+else()
+add_compile_flags_if_supported(-Wall)
+endif()
 add_compile_flags_if_supported(
--Wall -Wextra -W -Wwrite-strings
+-Wextra -W -Wwrite-strings
 -Wno-unused-parameter -Wno-long-long
 -Werror=return-type)
 if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")

[PATCH] D51867: [Diagnostics][NFC] Add error handling to FormatDiagnostic()

2018-09-10 Thread Jan Korous via Phabricator via cfe-commits 
jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

There seem to be couple implicit assumptions that might be better represented 
explicitly by asserts.


Repository:
  rC Clang

https://reviews.llvm.org/D51867

Files:
  lib/Basic/Diagnostic.cpp


Index: lib/Basic/Diagnostic.cpp
===
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -768,6 +768,7 @@
 void Diagnostic::
 FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
  SmallVectorImpl ) const {
+  assert(DiagStr <= DiagEnd && "Invalid DiagStr-DiagEnd range");
   // When the diagnostic string is only "%0", the entire string is being given
   // by an outside source.  Remove unprintable characters from this string
   // and skip all the other string processing.
@@ -798,17 +799,22 @@
 if (getArgKind(i) == DiagnosticsEngine::ak_qualtype)
   QualTypeVals.push_back(getRawArg(i));
 
-  while (DiagStr != DiagEnd) {
+  assert(DiagStr != nullptr && "DiagStr is nullptr");
+
+  while (DiagStr < DiagEnd) {
 if (DiagStr[0] != '%') {
   // Append non-%0 substrings to Str if we have one.
   const char *StrEnd = std::find(DiagStr, DiagEnd, '%');
   OutStr.append(DiagStr, StrEnd);
   DiagStr = StrEnd;
   continue;
-} else if (isPunctuation(DiagStr[1])) {
+} else if ((DiagStr + 1) < DiagEnd && isPunctuation(DiagStr[1])) {
   OutStr.push_back(DiagStr[1]);  // %% -> %.
   DiagStr += 2;
   continue;
+} else if ((DiagStr + 1) >= DiagEnd) {
+  llvm_unreachable("DiagStr ends with '%'");
+  return;
 }
 
 // Skip the %.
@@ -825,34 +831,40 @@
 // Check to see if we have a modifier.  If so eat it.
 if (!isDigit(DiagStr[0])) {
   Modifier = DiagStr;
-  while (DiagStr[0] == '-' ||
- (DiagStr[0] >= 'a' && DiagStr[0] <= 'z'))
+  while (DiagStr < DiagEnd &&
+ (DiagStr[0] == '-' || (DiagStr[0] >= 'a' && DiagStr[0] <= 'z')))
 ++DiagStr;
   ModifierLen = DiagStr-Modifier;
 
   // If we have an argument, get it next.
   if (DiagStr[0] == '{') {
 ++DiagStr; // Skip {.
+assert(DiagStr < DiagEnd && "Invalid DiagStr");
 Argument = DiagStr;
 
 DiagStr = ScanFormat(DiagStr, DiagEnd, '}');
 assert(DiagStr != DiagEnd && "Mismatched {}'s in diagnostic string!");
 ArgumentLen = DiagStr-Argument;
 ++DiagStr;  // Skip }.
+assert(DiagStr < DiagEnd && "Invalid DiagStr");
   }
 }
 
 assert(isDigit(*DiagStr) && "Invalid format for argument in diagnostic");
-unsigned ArgNo = *DiagStr++ - '0';
+
+unsigned ArgNo = *DiagStr - '0';
 
 // Only used for type diffing.
 unsigned ArgNo2 = ArgNo;
+++DiagStr;
 
 DiagnosticsEngine::ArgumentKind Kind = getArgKind(ArgNo);
 if (ModifierIs(Modifier, ModifierLen, "diff")) {
+  assert(DiagStr + 1 < DiagEnd && "Invalid diff modifier in DiagStr");
   assert(*DiagStr == ',' && isDigit(*(DiagStr + 1)) &&
  "Invalid format for diff modifier");
   ++DiagStr;  // Comma.
+  assert(DiagStr < DiagEnd && "Invalid DiagStr");
   ArgNo2 = *DiagStr++ - '0';
   DiagnosticsEngine::ArgumentKind Kind2 = getArgKind(ArgNo2);
   if (Kind == DiagnosticsEngine::ak_qualtype &&


Index: lib/Basic/Diagnostic.cpp
===
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -768,6 +768,7 @@
 void Diagnostic::
 FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
  SmallVectorImpl ) const {
+  assert(DiagStr <= DiagEnd && "Invalid DiagStr-DiagEnd range");
   // When the diagnostic string is only "%0", the entire string is being given
   // by an outside source.  Remove unprintable characters from this string
   // and skip all the other string processing.
@@ -798,17 +799,22 @@
 if (getArgKind(i) == DiagnosticsEngine::ak_qualtype)
   QualTypeVals.push_back(getRawArg(i));
 
-  while (DiagStr != DiagEnd) {
+  assert(DiagStr != nullptr && "DiagStr is nullptr");
+
+  while (DiagStr < DiagEnd) {
 if (DiagStr[0] != '%') {
   // Append non-%0 substrings to Str if we have one.
   const char *StrEnd = std::find(DiagStr, DiagEnd, '%');
   OutStr.append(DiagStr, StrEnd);
   DiagStr = StrEnd;
   continue;
-} else if (isPunctuation(DiagStr[1])) {
+} else if ((DiagStr + 1) < DiagEnd && isPunctuation(DiagStr[1])) {
   OutStr.push_back(DiagStr[1]);  // %% -> %.
   DiagStr += 2;
   continue;
+} else if ((DiagStr + 1) >= DiagEnd) {
+  llvm_unreachable("DiagStr ends with '%'");
+  return;
 }
 
 // Skip the %.
@@ -825,34 +831,40 @@
 // Check to see if we have a modifier.  If so eat it.
 if (!isDigit(DiagStr[0])) {
   Modifier = DiagStr;
-  while (DiagStr[0] == '-' ||
-

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth updated this revision to Diff 164690.
JonasToth added a comment.

- Fix typedependant expressions are ignored

This one took me way longer then it should have, but I discovered new templated 
code constructs and added test accordingly.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -2,6 +2,7 @@
 
 namespace std {
 class exception {};
+class invalid_argument : public exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
@@ -36,38 +37,38 @@
   try {
 throw non_derived_exception();
 // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-NOTES: 10:1: note: type defined here
   } catch (non_derived_exception ) {
   }
   throw non_derived_exception();
   // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-  // CHECK-NOTES: 9:1: note: type defined here
+  // CHECK-NOTES: 10:1: note: type defined here
 
 // FIXME: More complicated kinds of inheritance should be checked later, but there is
 // currently no way use ASTMatchers for this kind of task.
 #if 0
   // Handle private inheritance cases correctly.
   try {
 throw bad_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 10:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 11:1: note: type defined here
 throw no_good_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 11:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 12:1: note: type defined here
 throw really_creative();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-// CHECK MESSAGES: 12:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+// CHECK NOTES: 13:1: note: type defined here
   } catch (...) {
   }
   throw bad_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 10:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 11:1: note: type defined here
   throw no_good_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 11:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 12:1: note: type defined here
   throw really_creative();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-  // CHECK MESSAGES: 12:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+  // CHECK NOTES: 13:1: note: type defined here
 #endif
 }
 
@@ -91,24 +92,40 @@
   throw deep_hierarchy(); // Ok
 
   try {
-throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+throw terrible_idea();  // Ok, but multiple inheritance isn't clean
   } catch (std::exception ) { // Can be caught as std::exception, even with multiple inheritance
   }
   throw terrible_idea(); // Ok, but multiple inheritance
 }
 
+void test_lambdas() {
+  auto BadLambda = []() { throw int(42); };
+  // CHECK-NOTES: [[@LINE-1]]:33: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+  auto GoodLambda = []() { throw derived_exception(); };
+}
+
 // Templated function that throws exception based on template type
 template 
 void ThrowException() { throw T(); }
 // CHECK-NOTES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-09-10 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

@shuaiwang What are you working on next? Do you have an idea on how we
will handle the pointee of a pointer? That would be very interesting for
my const-correctness check. if you need help with anything, just say so.
I would like to support

Am 10.09.2018 um 18:49 schrieb Shuai Wang via Phabricator:

> shuaiwang added a comment.
> 
> In https://reviews.llvm.org/D45679#1183116, @george.karpenkov wrote:
> 
>> @aaron.ballman @alexfh @shuaiwang Would it be possible to move that code 
>> into a matcher, or into a something which could be used from Clang? We would 
>> also like to use similar functionality, but can't include stuff from 
>> clang-tidy.
> 
> FYI I haven't forgot about this, I'll look into doing it after a few pending 
> changes are in.
> 
> 
>  Comment at: clang-tidy/utils/ExprMutationAnalyzer.h:38
>  +  const Stmt *findDeclMutation(ArrayRef Matches);
>  +  const Stmt *findDeclMutation(const Decl *Dec);
>  +
> 
>  
> 
> lebedev.ri wrote:
> 
>> lebedev.ri wrote:
>> 
>>> lebedev.ri wrote:
>>> 
 @shuaiwang, @JonasToth hi.
  Is it very intentional that this `findDeclMutation()` is private?
 
 Is there some other way i should be doing if i have a statement `S`,
  a declRefExpr `D`, and i want to find the statement `Q`, which mutates
  the underlying variable to which the declRefExpr `D` refers?
>>> 
>>> (the statement `Q` within the statement `S`, of course)
>> 
>> @shuaiwang after a disscussion about this in IRC with @JonasToth, i have 
>> filed https://bugs.llvm.org/show_bug.cgi?id=3
>>  But i'm failing to CC you there. Are you not registered in the bugzilla?
> 
> There's no real reason findDeclMutation is private other than that there 
> wasn't a use case :)
>  Feel free to make it public if you find it useful that way.
> 
> I'll take a look at the bug, thanks for reporting it!
>  (I don't have an account there yet, I'm requesting one right now, will 
> follow up in the bug)
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D45679


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

2018-09-10 Thread Shuai Wang via Phabricator via cfe-commits 
shuaiwang added a comment.

In https://reviews.llvm.org/D45679#1183116, @george.karpenkov wrote:

> @aaron.ballman @alexfh @shuaiwang Would it be possible to move that code into 
> a matcher, or into a something which could be used from Clang? We would also 
> like to use similar functionality, but can't include stuff from clang-tidy.


FYI I haven't forgot about this, I'll look into doing it after a few pending 
changes are in.




Comment at: clang-tidy/utils/ExprMutationAnalyzer.h:38
+  const Stmt *findDeclMutation(ArrayRef Matches);
+  const Stmt *findDeclMutation(const Decl *Dec);
+

lebedev.ri wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > @shuaiwang, @JonasToth hi.
> > > Is it very intentional that this `findDeclMutation()` is private?
> > > 
> > > Is there some other way i should be doing if i have a statement `S`,
> > > a declRefExpr `D`, and i want to find the statement `Q`, which mutates
> > > the underlying variable to which the declRefExpr `D` refers?
> > (the statement `Q` within the statement `S`, of course)
> @shuaiwang after a disscussion about this in IRC with @JonasToth, i have 
> filed https://bugs.llvm.org/show_bug.cgi?id=3
> But i'm failing to CC you there. Are you not registered in the bugzilla?
There's no real reason findDeclMutation is private other than that there wasn't 
a use case :)
Feel free to make it public if you find it useful that way.

I'll take a look at the bug, thanks for reporting it!
(I don't have an account there yet, I'm requesting one right now, will follow 
up in the bug)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

2018-09-10 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits 
rogfer01 added a comment.

Thanks for reopening this @kristina.

I suggest passing `--sysroot=` to make sure we see the expected behaviour when 
the sysroot is actually empty.

Note that this would not really test the scenario where `DEFAULT_SYSROOT` is 
empty **and** no `--sysroot` appears in the command line. I'm not sure if we 
really want to test that case (but if we do, I think we will have to move that 
case into a test of its own and add a //feature// in `lit.cfg.py` that 
describes that clang does not have any built-in default sysroot).

Thoughts?


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

2018-09-10 Thread Sam McCall via Phabricator via cfe-commits 
sammccall added a comment.

Why this change?

(If you're changing it, `Optional` is probably clearer)


https://reviews.llvm.org/D51860



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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

2018-09-10 Thread Sam McCall via Phabricator via cfe-commits 
sammccall added a comment.

In https://reviews.llvm.org/D51747#1228919, @ilya-biryukov wrote:

> > Most of the value of adding an option is: if someone complains, we can tell 
> > them to go away :-) One possible corollary is: we shouldn't add the option 
> > until someone complains. I'm not 100% sure about that, though - not totally 
> > opposed to an option here.
>
> Any thoughts on tampering with provided compile args in the first place? Is 
> it fine for clangd to be opinionated about the warnings we choose to always 
> show to the users?
>  E.g. I'm a big fan of various uninitialized warnings, but nevertheless don't 
> think clangd should force them on everyone.


A few thoughts here:

- does CDB describe user or project preferences? unclear.
- "show this warning for code I build" is a higher bar than "show this warning 
for code I edit". So CDB probably enables too few warnings.
- Some warnings play well with -Werror (like uninit warnings), some don't (like 
deprecated). -Werror projects often disable interesting warnings.

I'm not sure we should strictly follow the CDB, but the bar to override it 
should probably be high.
Maybe the (default) behavior here should be "add -Wdeprecated 
-Wno-error=deprecated if -Werror is set"?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

  1   2   3   >