[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:78
+// function decl. Skips local symbols.
+llvm::DenseSet getNonLocalDeclRefs(const FunctionDecl *FD,
+ ParsedAST &AST) {

hokein wrote:
> thinking this a bit more, the function is non-trivial, we probably want unit 
> test for it.
agreed, sent out D67748



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:113
+// Checks the decls mentioned in Source are visible in the context of Target.
+// Achives that by performing lookups in Target's DeclContext.
+// Unfortunately these results need to be post-filtered since the AST we get is

hokein wrote:
> hmm, looks like we don't use TargetContext in the implemenation at all. Is 
> the comment out-of-date now?
we are still checking its visibility in target context, but without making use 
of declcontext. Updating the comment.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:160
+// usually be the first declaration in current translation unit with the
+// exception of template specialization. For those we return the previous
+// declaration instead of the first one, since it will be template decl itself

hokein wrote:
> function template is tricky, I may be not aware of the context, what's our 
> plan for supporting it? could you give an concrete example? it seems that the 
> current unit test doesn't cover it.
Yes we plan to support them, added a test case thanks for pointing it out.

This basically ensures that we select the correct decl in the case of template 
spec decls, because canonical decl is function template itself, instead of the 
specialization decl.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:183
+/// a.h:
+///   void foo() { return ; }
+///

hokein wrote:
> now we get a potential ODR violation in this example, maybe choose a 
> different example?
You are right, but we cannot get away from it by changing the example. Adding 
an "inline " instead, and I believe that's what we should do if we are moving a 
function definition to a header.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:224
+
+  Expected apply(const Selection &Sel) override {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

hokein wrote:
> The tweak is under development, do we need to hide it until it is completed?
I wasn't planning to land it before all of its dependencies are LGTM'd, but 
marking it as hidden just in case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65433/new/

https://reviews.llvm.org/D65433



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


[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 220821.
kadircet marked 17 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65433/new/

https://reviews.llvm.org/D65433

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -24,6 +24,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -33,6 +34,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 
 using ::testing::AllOf;
 using ::testing::HasSubstr;
@@ -554,7 +557,6 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -648,6 +650,117 @@
   EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
   StartsWith("fail"));
 }
+
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+  class Bar {
+void baz();
+  };
+
+  [[void [[Bar::[[b^a^z() [[{
+return;
+  }
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+return;
+  }
+  )cpp");
+
+  EXPECT_UNAVAILABLE(R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+[[(void)(5+3);
+return;]]
+  }]]
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, NoForwardDecl) {
+  Header = "void bar();";
+  EXPECT_UNAVAILABLE(R"cpp(
+  void bar() {
+return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+return;
+  })cpp");
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo(int test);
+
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Internal symbol usage.
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+void bar();
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Becomes available after making symbol visible.
+  Header = "void bar();" + Header;
+  EXPECT_AVAILABLE(R"cpp(
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // FIXME: Move declaration below bar to make it visible.
+  EXPECT_UNAVAILABLE(R"cpp(
+void foo();
+void bar();
+
+void fo^o() {
+  bar();
+})cpp");
+
+  // Order doesn't matter within a class.
+  EXPECT_AVAILABLE(R"cpp(
+class Bar {
+  void foo();
+  void bar();
+};
+
+void Bar::fo^o() {
+  bar();
+})cpp");
+
+  // FIXME: Perform include insertion to make symbol visible.
+  ExtraFiles["a.h"] = "void bar();";
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+#include "a.h"
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, TemplateSpec) {
+  EXPECT_AVAILABLE(R"cpp(
+template  void foo();
+void bar();
+template <> void foo();
+
+template<> void f^oo() {
+  bar();
+})cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -10,8 +10,10 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
 
 #include "TestTU.h"
-#include "gtest/gtest.h"
+#include "llvm/ADT/StringMap.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -33,8 +35,6 @@
 //   EXPECT_UNAVAILABLE("auto ^X^ = ^foo();");
 // }
 class TweakTest : public ::testing::Test {
-  const char *TweakID;
-
 public:
   // Inputs are wrapped in file boilerplate before attempting to apply a tweak.
   // Context describes the type of boilerplate.
@@ -47,6 +47,8 @@
 Expression,
   };
 
+  llvm::StringMap ExtraFiles;
+
 protected:
   TweakTest(const char *TweakID) : TweakID(TweakID) {}
 
@@ -76,6 +78,8 @@
   // Returns a matcher that accepts marked code snippets where the tweak is
   // available at the marked range.
   ::testing::Matcher isAvailable() const;
+
+  const char *TweakID;
 };
 
 #define TWEAK_TEST(TweakID)

[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

2019-09-19 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore updated this revision to Diff 220823.
stephanemoore added a comment.

Restrict ojbc-missing-hash to Objective-C language variants and fix sorting of
release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67737/new/

https://reviews.llvm.org/D67737

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp
  clang-tools-extra/clang-tidy/objc/MissingHashCheck.h
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-missing-hash.rst
  clang-tools-extra/test/clang-tidy/objc-missing-hash.m

Index: clang-tools-extra/test/clang-tidy/objc-missing-hash.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-missing-hash.m
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s objc-missing-hash %t
+
+typedef _Bool BOOL;
+#define YES 1
+#define NO 0
+typedef unsigned int NSUInteger;
+typedef void *id;
+
+@interface NSObject
+- (NSUInteger)hash;
+- (BOOL)isEqual:(id)object;
+@end
+
+@interface MissingHash : NSObject
+@end
+
+@implementation MissingHash
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 'MissingHash' implements -isEqual: without implementing -hash [objc-missing-hash]
+
+- (BOOL)isEqual:(id)object {
+  return YES;
+}
+
+@end
+
+@interface HasHash : NSObject
+@end
+
+@implementation HasHash
+
+- (NSUInteger)hash {
+  return 0;
+}
+
+- (BOOL)isEqual:(id)object {
+  return YES;
+}
+
+@end
+
+@interface NSArray : NSObject
+@end
+
+@interface MayHaveInheritedHash : NSArray
+@end
+
+@implementation MayHaveInheritedHash
+
+- (BOOL)isEqual:(id)object {
+  return YES;
+}
+
+@end
+
+@interface AnotherRootClass
+@end
+
+@interface NotDerivedFromNSObject : AnotherRootClass
+@end
+
+@implementation NotDerivedFromNSObject
+
+- (BOOL)isEqual:(id)object {
+  return NO;
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-missing-hash.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-missing-hash.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - objc-missing-hash
+
+objc-missing-hash
+=
+
+Finds Objective-C implementations that implement ``-isEqual:`` without also
+appropriately implementing ``-hash``.
+
+Apple documentation highlights that objects that are equal must have the same
+hash value:
+https://developer.apple.com/documentation/objectivec/1418956-nsobject/1418795-isequal?language=objc
+
+Note that the check only verifies the presence of ``-hash`` in scenarios where
+its omission could result in unexpected behavior. The verification of the
+implementation of ``-hash`` is the responsibility of the developer, e.g.,
+through the addition of unit tests to verify the implementation.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -325,6 +325,7 @@
objc-avoid-nserror-init
objc-avoid-spinlock
objc-forbidden-subclassing
+   objc-missing-hash
objc-property-declaration
objc-super-self
openmp-exception-escape
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
+- New :doc:`objc-missing-hash
+  ` check.
+
+  Finds Objective-C implementations that implement ``-isEqual:`` without also
+  appropriately implementing ``-hash``.
+
 - Improved :doc:`bugprone-posix-return
   ` check.
 
Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "AvoidNSErrorInitCheck.h"
 #include "AvoidSpinlockCheck.h"
 #include "ForbiddenSubclassingCheck.h"
+#include "MissingHashCheck.h"
 #include "PropertyDeclarationCheck.h"
 #include "SuperSelfCheck.h"
 
@@ -30,6 +31,8 @@
 "objc-avoid-spinlock");
 CheckFactories.registerCheck(
 "objc-forbidden-subclassing");
+CheckFactories.registerCheck(
+"objc-missing-hash");
 CheckFactories.registerCheck(
 "objc-property-declaration");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/objc/MissingHashCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/MissingHashCheck.h
@@ -0,0 +1,35 @@
+//===--- MissingHashCheck.h - clang-tidy *- 

[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-19 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: include/llvm/IR/DerivedTypes.h:500
+default:
+  assert(0 && "Cannot create narrower fp vector element type");
+  break;

nit: Use `llvm_unreachable("message")` instead of `assert(0 && "message")`


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67549/new/

https://reviews.llvm.org/D67549



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


[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

2019-09-19 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore marked 4 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp:40
+void MissingHashCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  objcMethodDecl(

Eugene.Zelenko wrote:
> Should check if language is Objective-C. See ForbiddenSubclassingCheck.cpp as 
> example. Will be good ensure that all Objective-C checks do this.
Thanks for calling this out; I tried using the check addition script and forgot 
to include this important conditional. I believe this has been resolved with my 
proposed changes.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:105
 
+- New :doc:`objc-missing-hash
+  ` check.

Eugene.Zelenko wrote:
> Wrong place. Please move to new checks list (in alphabetical order).
Thanks for pointing this out. I believe I may have made an error while 
resolving merge conflicts. I believe this has now been resolved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67737/new/

https://reviews.llvm.org/D67737



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


[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

2019-09-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

I think we're missing the client/server capability implementation in this 
patch, we should include them.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1131
+Callback> Reply) {
+  if (Params.positions.size() != 1) {
+elog("{0} positions provided to SelectionRange. Supports exactly one "

maybe add an `assert(!Params.positions.empty())`. I think we should not run 
into this case.



Comment at: clang-tools-extra/clangd/Protocol.h:1234
+
+struct SelectionRange {
+  // The semantic ranges for a position. Any range must contain all the 
previous

The data structures defined in `Protocol` are mirrored to LSP's. I think we 
should follow it for semantic selection as well (even though LSP use a wired 
structure, linked list).

so here, the structure should be like 

```
struct SelectionRange {
/**
 * The [range](#Range) of this selection range.
 */
Range range;
/**
 * The parent selection range containing this range. Therefore 
`parent.range` must contain `this.range`.
 */
llvm::Optional parent;
}
```

And we'd need to define a `render` function in `ClangLSPServer` to covert the 
`vector` to the LSP's `SelectionRange`.



Comment at: clang-tools-extra/clangd/Protocol.h:1241
+llvm::json::Value toJSON(const SelectionRange &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SelectionRange &);
+

does this operator get used in this patch?



Comment at: clang-tools-extra/clangd/test/selection-range.test:4
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void
 func() {\n int var1;\n int var2 = var1;\n}"}}}
+---

could we create a smaller test? just `void func() {}` is enough, I'd keep the 
lit test as small as possible (since this feature has been test in the unittest)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67720/new/

https://reviews.llvm.org/D67720



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


[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

2019-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1131
+Callback> Reply) {
+  if (Params.positions.size() != 1) {
+elog("{0} positions provided to SelectionRange. Supports exactly one "

hokein wrote:
> maybe add an `assert(!Params.positions.empty())`. I think we should not run 
> into this case.
But `Params` comes to clangd over LSP, right?
That means `assert` can fire in case of bad inputs over LSP to clangd.
Bad inputs over LSP should never crash clangd.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67720/new/

https://reviews.llvm.org/D67720



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


[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:158
+  //  i.e: a::Bar> instead of a::Bar>
+  printTemplateArgumentList(OS, TSTL.getTypePtr()->template_arguments(),
+TD->getASTContext().getPrintingPolicy());

Are we trying to replace the whole name?

We can avoid this problem by not qualifying template arguments in the first 
place.
I.e. instead of replacing the whole name, including template arguments:
```
[[baz]] -> [[::ns::baz]]
```
We could simply replace the part before template arguments:
```
[[baz]] -> [[::ns::baz]]
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66647/new/

https://reviews.llvm.org/D66647



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


[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

2019-09-19 Thread Mateusz Furdyna via Phabricator via cfe-commits
furdyna created this revision.
furdyna added reviewers: rsmith, ioeric.
Herald added a project: clang.
furdyna edited the summary of this revision.

By additional regex match, grouping of main include can be enabled in files 
that are not normally considered as a C/C++ source code.
For example, this might be useful in templated code, where template 
implementations are being held in *Impl.hpp files.
On the occassion, 'assume-filename' option description was reworded as it was 
misleading. It has nothing to do with `style=file` option and it does not 
influence sourced style filename.


Repository:
  rC Clang

https://reviews.llvm.org/D67750

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -395,6 +395,57 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, LeavesMainHeaderFirstInAdditionalExtensions) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?|(Impl)?$";
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+
+  // .cpp extension is considered "main" by default
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.cpp"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.cpp"));
+
+  // Allow additional filenames / extensions
+  Style.IncludeIsMainSourceRegex = "(Impl\\.hpp)|(\\.xxx)$";
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+}
+
 TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12194,6 +12194,8 @@
   IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
   "abc$");
+  CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
+  IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
   Style.RawStringFormats.clear();
   std::vector ExpectedRawStringFormats = {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -72,12 +72,12 @@
   cl::init(clang::format::DefaultFallbackStyle),
   cl::cat(ClangFormatCategory));
 
-static cl::opt
-AssumeFileName("assume-filename",
-   cl::desc("When reading from stdin, clang-format assumes this\n"
-"filename to look for a style config file (with\n"
-"-style=file) and to determine the language."),
-   cl::init(""), cl::cat(ClangFormatCategory));
+static cl::opt AssumeFileName(
+"assume-filename",
+cl::desc("Override filename used to determine the language.\n"
+ "When reading from stdin, clang-format assumes this\n"
+ "filename to determine the language."),
+cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt Inplace("i",
  cl::des

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

2019-09-19 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
ostannard added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63932/new/

https://reviews.llvm.org/D63932



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


r372307 - [TestCommit] Trivial change to test commit access.

2019-09-19 Thread Mark Murray via cfe-commits
Author: markrvmurray
Date: Thu Sep 19 02:24:42 2019
New Revision: 372307

URL: http://llvm.org/viewvc/llvm-project?rev=372307&view=rev
Log:
[TestCommit] Trivial change to test commit access.

Modified:
cfe/trunk/bindings/python/README.txt

Modified: cfe/trunk/bindings/python/README.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/README.txt?rev=372307&r1=372306&r2=372307&view=diff
==
--- cfe/trunk/bindings/python/README.txt (original)
+++ cfe/trunk/bindings/python/README.txt Thu Sep 19 02:24:42 2019
@@ -1,5 +1,5 @@
 
//===--===//
-// Clang Python Bindings.
+// Clang Python Bindings
 
//===--===//
 
 This directory implements Python bindings for Clang.


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


r372306 - [TestCommit] Trivial change to test commit access.

2019-09-19 Thread Mark Murray via cfe-commits
Author: markrvmurray
Date: Thu Sep 19 02:02:12 2019
New Revision: 372306

URL: http://llvm.org/viewvc/llvm-project?rev=372306&view=rev
Log:
[TestCommit] Trivial change to test commit access.

Modified:
cfe/trunk/bindings/python/README.txt

Modified: cfe/trunk/bindings/python/README.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/README.txt?rev=372306&r1=372305&r2=372306&view=diff
==
--- cfe/trunk/bindings/python/README.txt (original)
+++ cfe/trunk/bindings/python/README.txt Thu Sep 19 02:02:12 2019
@@ -1,5 +1,5 @@
 
//===--===//
-// Clang Python Bindings
+// Clang Python Bindings.
 
//===--===//
 
 This directory implements Python bindings for Clang.


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


[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

2019-09-19 Thread Mirko Brkusanin via Phabricator via cfe-commits
mbrkusanin added a comment.

Any comment on whether we should split this into two patches? One that adds 
`MCTargetOptions` to `MCAsmInfo` and another one that just fixes prefixes for 
Mips.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66795/new/

https://reviews.llvm.org/D66795



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


[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

2019-09-19 Thread Mirko Brkusanin via Phabricator via cfe-commits
mbrkusanin updated this revision to Diff 220830.
mbrkusanin added a comment.

- `MipsMCAsmInfo()` now always reads ABI from `MipsABIInfo` instead of `Triple`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66795/new/

https://reviews.llvm.org/D66795

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h
  llvm/lib/Target/ARC/MCTargetDesc/ARCMCTargetDesc.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
  llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
  llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.h
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
  llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
  llvm/lib/Target/Lanai/MCTargetDesc/LanaiMCAsmInfo.cpp
  llvm/lib/Target/Lanai/MCTargetDesc/LanaiMCAsmInfo.h
  llvm/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
  llvm/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.h
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.h
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCTargetDesc.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.h
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
  llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
  llvm/lib/Target/XCore/MCTargetDesc/XCoreMCTargetDesc.cpp
  llvm/test/CodeGen/Mips/compactbranches/no-beqzc-bnezc.ll
  llvm/test/MC/Mips/macro-li.d.s
  llvm/test/MC/Mips/macro-li.s.s
  llvm/test/MC/Mips/private-prefix.s
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/Disassembler.cpp
  llvm/tools/llvm-mc/Disassembler.h
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/ExecutionEngine/JITLink/JITLinkTestCommon.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/MCInstPrinter.cpp

Index: llvm/unittests/MC/MCInstPrinter.cpp
===
--- llvm/unittests/MC/MCInstPrinter.cpp
+++ llvm/unittests/MC/MCInstPrinter.cpp
@@ -9,6 +9,7 @@
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
@@ -40,7 +41,8 @@
   return;
 
 MRI.reset(TheTarget->createMCRegInfo(TripleName));
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName));
+MCTargetOptions MCOptions;
+MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 MII.reset(TheTarget->createMCInstrInfo());
 Printer.reset(TheTarget->createMCInstPrinter(
 Triple(TripleName), MAI->getAssemblerDialect(), *MAI, *MII, *MRI));
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -12,6 +12,7 @@
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "gtest/gtest.h"
@@ -37,7 +38,8 @@
   return;
 
 MRI.reset(TheTarget->createMCRegInfo(Triple));
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple));
+MCTargetOptions MCOptions;
+MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple, MCOptions));
 Ctx = std::make_unique(MAI.get(), MRI.get(), nullptr);
   }
 
Index: llvm/unittests/ExecutionEngine/JITLink/JITLinkTestCommon.cpp
===

[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 220838.
yaxunl retitled this revision from "[CUDA][HIP] Diagnose defaulted constructor 
only if it is used" to "[CUDA][HIP] Fix hostness of defaulted constructor".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Posts a new fix for this issue, where the defaulted constructor definition 
follows the hostness of the original declaration in the class. Also fix the 
issue when defaulted ctor has explicit host device attribs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509

Files:
  lib/Sema/SemaCUDA.cpp
  test/SemaCUDA/default-ctor.cu


Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:-fcuda-is-device -verify -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only \
+// RUN:-verify -verify-ignore-unexpected=note %s
+
+#include "Inputs/cuda.h"
+
+struct In { In() = default; };
+struct InD { __device__ InD() = default; };
+struct InH { __host__ InH() = default; };
+struct InHD { __host__ __device__ InHD() = default; };
+
+struct Out { Out(); };
+struct OutD { __device__ OutD(); };
+struct OutH { __host__ OutH(); };
+struct OutHD { __host__ __device__ OutHD(); };
+
+Out::Out() = default;
+__device__ OutD::OutD() = default;
+__host__ OutH::OutH() = default;
+__host__ __device__ OutHD::OutHD() = default;
+
+__device__ void fd() {
+  In in;
+  InD ind;
+  InH inh; // expected-error{{no matching constructor for initialization of 
'InH'}}
+  InHD inhd;
+  Out out; // expected-error{{no matching constructor for initialization of 
'Out'}}
+  OutD outd;
+  OutH outh; // expected-error{{no matching constructor for initialization of 
'OutH'}}
+  OutHD outhd;
+}
+
+__host__ void fh() {
+  In in;
+  InD ind; // expected-error{{no matching constructor for initialization of 
'InD'}}
+  InH inh;
+  InHD inhd;
+  Out out;
+  OutD outd; // expected-error{{no matching constructor for initialization of 
'OutD'}}
+  OutH outh;
+  OutHD outhd;
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -267,6 +267,12 @@
CXXMethodDecl *MemberDecl,
bool ConstRHS,
bool Diagnose) {
+  bool InClass = MemberDecl->getLexicalParent() == MemberDecl->getParent();
+  bool hasAttr = MemberDecl->hasAttr() ||
+ MemberDecl->hasAttr();
+  if (!InClass || hasAttr)
+return false;
+
   llvm::Optional InferredTarget;
 
   // We're going to invoke special member lookup; mark that these special


Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:-fcuda-is-device -verify -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only \
+// RUN:-verify -verify-ignore-unexpected=note %s
+
+#include "Inputs/cuda.h"
+
+struct In { In() = default; };
+struct InD { __device__ InD() = default; };
+struct InH { __host__ InH() = default; };
+struct InHD { __host__ __device__ InHD() = default; };
+
+struct Out { Out(); };
+struct OutD { __device__ OutD(); };
+struct OutH { __host__ OutH(); };
+struct OutHD { __host__ __device__ OutHD(); };
+
+Out::Out() = default;
+__device__ OutD::OutD() = default;
+__host__ OutH::OutH() = default;
+__host__ __device__ OutHD::OutHD() = default;
+
+__device__ void fd() {
+  In in;
+  InD ind;
+  InH inh; // expected-error{{no matching constructor for initialization of 'InH'}}
+  InHD inhd;
+  Out out; // expected-error{{no matching constructor for initialization of 'Out'}}
+  OutD outd;
+  OutH outh; // expected-error{{no matching constructor for initialization of 'OutH'}}
+  OutHD outhd;
+}
+
+__host__ void fh() {
+  In in;
+  InD ind; // expected-error{{no matching constructor for initialization of 'InD'}}
+  InH inh;
+  InHD inhd;
+  Out out;
+  OutD outd; // expected-error{{no matching constructor for initialization of 'OutD'}}
+  OutH outh;
+  OutHD outhd;
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -267,6 +267,12 @@
CXXMethodDecl *MemberDecl,
bool ConstRHS,
bool Diagnose) {
+  bool InClass = MemberDecl->getLexicalParent() == MemberDecl->getPar

[PATCH] D67744: [clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.

2019-09-19 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision.
gribozavr added a comment.
This revision is now accepted and ready to land.

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67744/new/

https://reviews.llvm.org/D67744



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


[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-19 Thread Kerry McLaughlin via Phabricator via cfe-commits
kmclaughlin updated this revision to Diff 220845.
kmclaughlin marked 5 inline comments as done.
kmclaughlin added a reviewer: rovka.
kmclaughlin added a comment.

- Moved getNarrowerFpElementVectorType logic into getTruncatedElementVectorType
- Shared code which handles Subdivide2Argument and Subdivide4Argument where 
possible
- Replaced an unreachable with an assert in Function.cpp
- Replaced an assert with an unreachable in DerivedTypes.h


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67549/new/

https://reviews.llvm.org/D67549

Files:
  include/llvm/IR/DerivedTypes.h
  include/llvm/IR/Intrinsics.h
  include/llvm/IR/Intrinsics.td
  lib/IR/Function.cpp
  utils/TableGen/IntrinsicEmitter.cpp

Index: utils/TableGen/IntrinsicEmitter.cpp
===
--- utils/TableGen/IntrinsicEmitter.cpp
+++ utils/TableGen/IntrinsicEmitter.cpp
@@ -221,7 +221,9 @@
   IIT_STRUCT8 = 40,
   IIT_F128 = 41,
   IIT_VEC_ELEMENT = 42,
-  IIT_SCALABLE_VEC = 43
+  IIT_SCALABLE_VEC = 43,
+  IIT_SUBDIVIDE2_ARG = 44,
+  IIT_SUBDIVIDE4_ARG = 45
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -293,6 +295,10 @@
   Sig.push_back(IIT_PTR_TO_ELT);
 else if (R->isSubClassOf("LLVMVectorElementType"))
   Sig.push_back(IIT_VEC_ELEMENT);
+else if (R->isSubClassOf("LLVMSubdivide2VectorType"))
+  Sig.push_back(IIT_SUBDIVIDE2_ARG);
+else if (R->isSubClassOf("LLVMSubdivide4VectorType"))
+  Sig.push_back(IIT_SUBDIVIDE4_ARG);
 else
   Sig.push_back(IIT_ARG);
 return Sig.push_back((Number << 3) | 7 /*IITDescriptor::AK_MatchType*/);
Index: lib/IR/Function.cpp
===
--- lib/IR/Function.cpp
+++ lib/IR/Function.cpp
@@ -703,7 +703,9 @@
   IIT_STRUCT8 = 40,
   IIT_F128 = 41,
   IIT_VEC_ELEMENT = 42,
-  IIT_SCALABLE_VEC = 43
+  IIT_SCALABLE_VEC = 43,
+  IIT_SUBDIVIDE2_ARG = 44,
+  IIT_SUBDIVIDE4_ARG = 45
 };
 
 static void DecodeIITType(unsigned &NextElt, ArrayRef Infos,
@@ -868,6 +870,18 @@
   DecodeIITType(NextElt, Infos, OutputTable);
 return;
   }
+  case IIT_SUBDIVIDE2_ARG: {
+unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
+OutputTable.push_back(IITDescriptor::get(IITDescriptor::Subdivide2Argument,
+ ArgInfo));
+return;
+  }
+  case IIT_SUBDIVIDE4_ARG: {
+unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
+OutputTable.push_back(IITDescriptor::get(IITDescriptor::Subdivide4Argument,
+ ArgInfo));
+return;
+  }
   case IIT_VEC_ELEMENT: {
 unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
 OutputTable.push_back(IITDescriptor::get(IITDescriptor::VecElementArgument,
@@ -970,6 +984,15 @@
 assert(ITy->getBitWidth() % 2 == 0);
 return IntegerType::get(Context, ITy->getBitWidth() / 2);
   }
+  case IITDescriptor::Subdivide2Argument:
+  case IITDescriptor::Subdivide4Argument: {
+Type *Ty = Tys[D.getArgumentNumber()];
+VectorType *VTy = dyn_cast(Ty);
+assert(VTy && "Expected an argument of Vector Type");
+if (D.Kind == IITDescriptor::Subdivide2Argument)
+  return VectorType::getSubdividedVectorType(VTy, 1);
+return VectorType::getSubdividedVectorType(VTy, 2);
+  }
   case IITDescriptor::HalfVecArgument:
 return VectorType::getHalfElementsVectorType(cast(
   Tys[D.getArgumentNumber()]));
@@ -1269,6 +1292,23 @@
   auto *ReferenceType = dyn_cast(ArgTys[D.getArgumentNumber()]);
   return !ReferenceType || Ty != ReferenceType->getElementType();
 }
+case IITDescriptor::Subdivide2Argument:
+case IITDescriptor::Subdivide4Argument: {
+  // If this is a forward reference, defer the check for later.
+  if (D.getArgumentNumber() >= ArgTys.size())
+return IsDeferredCheck || DeferCheck(Ty);
+
+  Type *NewTy = ArgTys[D.getArgumentNumber()];
+  if (VectorType *VTy = dyn_cast(NewTy)) {
+if (D.Kind == IITDescriptor::Subdivide2Argument)
+  NewTy = VectorType::getSubdividedVectorType(VTy, 1);
+else
+  NewTy = VectorType::getSubdividedVectorType(VTy, 2);
+  } else
+return true;
+
+  return Ty != NewTy;
+}
 case IITDescriptor::ScalableVecArgument: {
   VectorType *VTy = dyn_cast(Ty);
   if (!VTy || !VTy->isScalable())
Index: include/llvm/IR/Intrinsics.td
===
--- include/llvm/IR/Intrinsics.td
+++ include/llvm/IR/Intrinsics.td
@@ -187,6 +187,12 @@
 // vector type, but change the element count to be half as many
 class LLVMHalfElementsVectorType : LLVMMatchType;
 
+// Match the type of another intrinsic parameter that is expected to be a
+// vector type (i.e. ) but with each element subdivided to
+// form a vector with more elements that are smaller than the original.
+class LLV

[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-19 Thread Kerry McLaughlin via Phabricator via cfe-commits
kmclaughlin added a comment.

Thanks for reviewing this patch, @rovka and @sdesmalen!




Comment at: include/llvm/IR/Intrinsics.h:130
+ Kind == PtrToElt || Kind == VecElementArgument ||
+ Kind == Subdivide2Argument || Kind == Subdivide4Argument);
   return Argument_Info >> 3;

rovka wrote:
> Not directly related to your change: Is there a reason why VecOfAnyPtrsToElt 
> isn't in this assert? If not, maybe this is a good time to add a 
> First/LastArgumentKind and just check that Kind is in the range.
I think it's probably best not to include VecOfAnyPtrsToElt here as it was 
removed from the assert when the functions //getOverloadArgNumber// and 
//getRefArgNumber// below were added (these should be used instead of 
getArgumentNumber in this case).



Comment at: lib/IR/Function.cpp:1312
+case IITDescriptor::Subdivide4Argument: {
+  // This may only be used when referring to a previous vector argument.
+  if (D.getArgumentNumber() >= ArgTys.size())

rovka wrote:
> Can you share this code? Either a small helper or a fallthrough with a repeat 
> check on D.Kind to get the number of subdivisions would work.
This code is now shared, as is the code to handle Subdivide2Argument & 
Subdivide4Argument in Function.cpp above.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67549/new/

https://reviews.llvm.org/D67549



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


[libunwind] r372316 - Creating release candidate final from release_900 branch

2019-09-19 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Thu Sep 19 06:10:55 2019
New Revision: 372316

URL: http://llvm.org/viewvc/llvm-project?rev=372316&view=rev
Log:
Creating release candidate final from release_900 branch

Added:
libunwind/tags/RELEASE_900/final/
  - copied from r372315, libunwind/branches/release_90/

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


[libclc] r372316 - Creating release candidate final from release_900 branch

2019-09-19 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Thu Sep 19 06:10:55 2019
New Revision: 372316

URL: http://llvm.org/viewvc/llvm-project?rev=372316&view=rev
Log:
Creating release candidate final from release_900 branch

Added:
libclc/tags/RELEASE_900/final/
  - copied from r372315, libclc/branches/release_90/

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


[clang-tools-extra] r372317 - [clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.

2019-09-19 Thread Yitzhak Mandelbaum via cfe-commits
Author: ymandel
Date: Thu Sep 19 06:12:05 2019
New Revision: 372317

URL: http://llvm.org/viewvc/llvm-project?rev=372317&view=rev
Log:
[clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit 
constructors.

Summary:
After revision 370919, this check incorrectly flags certain cases of implicit
constructors. Specifically, if an argument is annotated with an
argument-comment and the argument expression triggers an implicit constructor,
then the argument comment is associated with argument of the implicit
constructor.

However, this only happens when the constructor has more than one argument.
This revision fixes the check for implicit constructors and adds a regression
test for this case.

Note: r370919 didn't cause this bug, it simply uncovered it by fixing another
bug that was masking the behavior.

Reviewers: gribozavr

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=372317&r1=372316&r2=372317&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Thu 
Sep 19 06:12:05 2019
@@ -337,7 +337,7 @@ void ArgumentCommentCheck::check(const M
   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
   } else {
 const auto *Construct = cast(E);
-if (Construct->getNumArgs() == 1 &&
+if (Construct->getNumArgs() > 0 &&
 Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) 
{
   // Ignore implicit construction.
   return;

Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp?rev=372317&r1=372316&r2=372317&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp Thu 
Sep 19 06:12:05 2019
@@ -63,6 +63,28 @@ void ignores_underscores() {
   f3(/*With_Underscores=*/false);
 }
 
+namespace IgnoresImplicit {
+struct S {
+  S(int x);
+  int x;
+};
+
+struct T {
+  // Use two arguments (one defaulted) because simplistic check for implicit
+  // constructor looks for only one argument. We need to default the argument 
so
+  // that it will still be triggered implicitly.  This is not contrived -- it
+  // comes up in real code, for example std::set(std::initializer_list...).
+  T(S s, int y = 0);
+};
+
+void k(T arg1);
+
+void mynewtest() {
+  int foo = 3;
+  k(/*arg1=*/S(foo));
+}
+} // namespace IgnoresImplicit
+
 namespace ThisEditDistanceAboveThreshold {
 void f4(int xxx);
 void g() { f4(/*xyz=*/0); }


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


r372318 - [CUDA][HIP] Fix typo in `BestViableFunction`

2019-09-19 Thread Michael Liao via cfe-commits
Author: hliao
Date: Thu Sep 19 06:14:03 2019
New Revision: 372318

URL: http://llvm.org/viewvc/llvm-project?rev=372318&view=rev
Log:
[CUDA][HIP] Fix typo in `BestViableFunction`

Summary:
- Should consider viable ones only when checking SameSide candidates.
- Replace erasing with clearing viable flag to reduce data
  moving/copying.
- Add one and revise another one as the diagnostic message are more
  relevant compared to previous one.

Reviewers: tra

Subscribers: cfe-commits, yaxunl

Tags: #clang

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

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/SemaCUDA/function-overload.cu
cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=372318&r1=372317&r2=372318&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Sep 19 06:14:03 2019
@@ -9422,17 +9422,19 @@ OverloadCandidateSet::BestViableFunction
 const FunctionDecl *Caller = dyn_cast(S.CurContext);
 bool ContainsSameSideCandidate =
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
-  return Cand->Function &&
+  // Consider viable function only.
+  return Cand->Viable && Cand->Function &&
  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
  Sema::CFP_SameSide;
 });
 if (ContainsSameSideCandidate) {
-  auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
-return Cand->Function &&
-   S.IdentifyCUDAPreference(Caller, Cand->Function) ==
-   Sema::CFP_WrongSide;
-  };
-  llvm::erase_if(Candidates, IsWrongSideCandidate);
+  // Clear viable flag for WrongSide varible candidates.
+  llvm::for_each(Candidates, [&](OverloadCandidate *Cand) {
+if (Cand->Viable && Cand->Function &&
+S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+Sema::CFP_WrongSide)
+  Cand->Viable = false;
+  });
 }
   }
 

Modified: cfe/trunk/test/SemaCUDA/function-overload.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/function-overload.cu?rev=372318&r1=372317&r2=372318&view=diff
==
--- cfe/trunk/test/SemaCUDA/function-overload.cu (original)
+++ cfe/trunk/test/SemaCUDA/function-overload.cu Thu Sep 19 06:14:03 2019
@@ -402,3 +402,20 @@ __host__ void test_host_template_overloa
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ 
variant.
 }
+
+// Two classes with `operator-` defined. One of them is device only.
+struct C1;
+struct C2;
+__device__
+int operator-(const C1 &x, const C1 &y);
+int operator-(const C2 &x, const C2 &y);
+
+template 
+__host__ __device__ int constexpr_overload(const T &x, const T &y) {
+  return x - y;
+}
+
+// Verify that function overloading doesn't prune candidate wrongly.
+int test_constexpr_overload(C2 &x, C2 &y) {
+  return constexpr_overload(x, y);
+}

Modified: cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu?rev=372318&r1=372317&r2=372318&view=diff
==
--- cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu (original)
+++ cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu Thu Sep 
19 06:14:03 2019
@@ -74,11 +74,13 @@ struct B4_with_device_copy_ctor {
 struct C4_with_collision : A4_with_host_copy_ctor, B4_with_device_copy_ctor {
 };
 
-// expected-note@-3 {{copy constructor of 'C4_with_collision' is implicitly 
deleted because base class 'B4_with_device_copy_ctor' has no copy constructor}}
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) 
not viable: call to invalid function from __host__ function}}
+// expected-note@-4 {{implicit copy constructor inferred target collision: 
call to both __host__ and __device__ members}}
+// expected-note@-5 {{candidate constructor (the implicit default constructor) 
not viable: requires 0 arguments, but 1 was provided}}
 
 void hostfoo4() {
   C4_with_collision c;
-  C4_with_collision c2 = c; // expected-error {{call to implicitly-deleted 
copy constructor of 'C4_with_collision'}}
+  C4_with_collision c2 = c; // expected-error {{no matching constructor for 
initialization of 'C4_with_collision'}}
 }
 
 
//--


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


[PATCH] D67730: [CUDA][HIP] Fix typo in `BestViableFunction`

2019-09-19 Thread Michael Liao via Phabricator via cfe-commits
hliao marked 3 inline comments as done.
hliao added a comment.

r372318 with test case revised following suggestion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67730/new/

https://reviews.llvm.org/D67730



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


[PATCH] D67730: [CUDA][HIP] Fix typo in `BestViableFunction`

2019-09-19 Thread Michael Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372318: [CUDA][HIP] Fix typo in `BestViableFunction` 
(authored by hliao, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67730?vs=220738&id=220854#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67730/new/

https://reviews.llvm.org/D67730

Files:
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/SemaCUDA/function-overload.cu
  cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu


Index: cfe/trunk/test/SemaCUDA/function-overload.cu
===
--- cfe/trunk/test/SemaCUDA/function-overload.cu
+++ cfe/trunk/test/SemaCUDA/function-overload.cu
@@ -402,3 +402,20 @@
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ 
variant.
 }
+
+// Two classes with `operator-` defined. One of them is device only.
+struct C1;
+struct C2;
+__device__
+int operator-(const C1 &x, const C1 &y);
+int operator-(const C2 &x, const C2 &y);
+
+template 
+__host__ __device__ int constexpr_overload(const T &x, const T &y) {
+  return x - y;
+}
+
+// Verify that function overloading doesn't prune candidate wrongly.
+int test_constexpr_overload(C2 &x, C2 &y) {
+  return constexpr_overload(x, y);
+}
Index: cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
===
--- cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
+++ cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
@@ -74,11 +74,13 @@
 struct C4_with_collision : A4_with_host_copy_ctor, B4_with_device_copy_ctor {
 };
 
-// expected-note@-3 {{copy constructor of 'C4_with_collision' is implicitly 
deleted because base class 'B4_with_device_copy_ctor' has no copy constructor}}
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) 
not viable: call to invalid function from __host__ function}}
+// expected-note@-4 {{implicit copy constructor inferred target collision: 
call to both __host__ and __device__ members}}
+// expected-note@-5 {{candidate constructor (the implicit default constructor) 
not viable: requires 0 arguments, but 1 was provided}}
 
 void hostfoo4() {
   C4_with_collision c;
-  C4_with_collision c2 = c; // expected-error {{call to implicitly-deleted 
copy constructor of 'C4_with_collision'}}
+  C4_with_collision c2 = c; // expected-error {{no matching constructor for 
initialization of 'C4_with_collision'}}
 }
 
 
//--
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -9422,17 +9422,19 @@
 const FunctionDecl *Caller = dyn_cast(S.CurContext);
 bool ContainsSameSideCandidate =
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
-  return Cand->Function &&
+  // Consider viable function only.
+  return Cand->Viable && Cand->Function &&
  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
  Sema::CFP_SameSide;
 });
 if (ContainsSameSideCandidate) {
-  auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
-return Cand->Function &&
-   S.IdentifyCUDAPreference(Caller, Cand->Function) ==
-   Sema::CFP_WrongSide;
-  };
-  llvm::erase_if(Candidates, IsWrongSideCandidate);
+  // Clear viable flag for WrongSide varible candidates.
+  llvm::for_each(Candidates, [&](OverloadCandidate *Cand) {
+if (Cand->Viable && Cand->Function &&
+S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+Sema::CFP_WrongSide)
+  Cand->Viable = false;
+  });
 }
   }
 


Index: cfe/trunk/test/SemaCUDA/function-overload.cu
===
--- cfe/trunk/test/SemaCUDA/function-overload.cu
+++ cfe/trunk/test/SemaCUDA/function-overload.cu
@@ -402,3 +402,20 @@
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ variant.
 }
+
+// Two classes with `operator-` defined. One of them is device only.
+struct C1;
+struct C2;
+__device__
+int operator-(const C1 &x, const C1 &y);
+int operator-(const C2 &x, const C2 &y);
+
+template 
+__host__ __device__ int constexpr_overload(const T &x, const T &y) {
+  return x - y;
+}
+
+// Verify that function overloading doesn't prune candidate wrongly.
+int test_constexpr_overload(C2 &x, C2 &y) {
+  return constexpr_overload(x, y);
+}
Index: cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
===

[PATCH] D67744: [clang-tidy] Fix bugprone-argument-comment-check to correctly ignore implicit constructors.

2019-09-19 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372317: [clang-tidy] Fix bugprone-argument-comment-check to 
correctly ignore implicit… (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67744?vs=220787&id=220853#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67744/new/

https://reviews.llvm.org/D67744

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp


Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
@@ -63,6 +63,28 @@
   f3(/*With_Underscores=*/false);
 }
 
+namespace IgnoresImplicit {
+struct S {
+  S(int x);
+  int x;
+};
+
+struct T {
+  // Use two arguments (one defaulted) because simplistic check for implicit
+  // constructor looks for only one argument. We need to default the argument 
so
+  // that it will still be triggered implicitly.  This is not contrived -- it
+  // comes up in real code, for example std::set(std::initializer_list...).
+  T(S s, int y = 0);
+};
+
+void k(T arg1);
+
+void mynewtest() {
+  int foo = 3;
+  k(/*arg1=*/S(foo));
+}
+} // namespace IgnoresImplicit
+
 namespace ThisEditDistanceAboveThreshold {
 void f4(int xxx);
 void g() { f4(/*xyz=*/0); }
Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -337,7 +337,7 @@
   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
   } else {
 const auto *Construct = cast(E);
-if (Construct->getNumArgs() == 1 &&
+if (Construct->getNumArgs() > 0 &&
 Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) 
{
   // Ignore implicit construction.
   return;


Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment.cpp
@@ -63,6 +63,28 @@
   f3(/*With_Underscores=*/false);
 }
 
+namespace IgnoresImplicit {
+struct S {
+  S(int x);
+  int x;
+};
+
+struct T {
+  // Use two arguments (one defaulted) because simplistic check for implicit
+  // constructor looks for only one argument. We need to default the argument so
+  // that it will still be triggered implicitly.  This is not contrived -- it
+  // comes up in real code, for example std::set(std::initializer_list...).
+  T(S s, int y = 0);
+};
+
+void k(T arg1);
+
+void mynewtest() {
+  int foo = 3;
+  k(/*arg1=*/S(foo));
+}
+} // namespace IgnoresImplicit
+
 namespace ThisEditDistanceAboveThreshold {
 void f4(int xxx);
 void g() { f4(/*xyz=*/0); }
Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -337,7 +337,7 @@
   llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
   } else {
 const auto *Construct = cast(E);
-if (Construct->getNumArgs() == 1 &&
+if (Construct->getNumArgs() > 0 &&
 Construct->getArg(0)->getSourceRange() == Construct->getSourceRange()) {
   // Ignore implicit construction.
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-09-19 Thread Peter Waller via Phabricator via cfe-commits
peterwaller-arm updated this revision to Diff 220856.
peterwaller-arm marked 14 inline comments as done.
peterwaller-arm edited the summary of this revision.
peterwaller-arm added a comment.

Thanks everyone for your comments.

Changes since last patch:

- Reintroduce handling of (no phase arg specified), -S, -emit-llvm, etc.

  The code implementing it was reordered for clarity.

  After internal discussion we concluded it was preferable to land these now 
even though they are unimplemented on the frontend side, in order to avoid 
giving assertion errors for unimplemented behaviour.

  It is planned that the frontend will emit sensible errors for unimplemented 
behaviour.

- Fixes to the cover letter for --driver-mode=fortran => --driver-mode=flang

- Add a test (flang-not-installed.f90) which demonstrates the error message if 
clang is invoked with no flang available, by unsetting PATH. This is a 
non-"-###" test which also does not depend on the presence of flang.
  - Since it uses shell syntax, it is marked unsupported on windows.

- In response to a private comment, Flang mode now coerces TY_PP_Fortran to 
TY_Fortran. This:
  - Leaves the legacy behaviour unchanged
  - Ensures that we do not emit a warning when passing TY_PP_Fortran inputs to 
-E
  - Should ensure that TY_PP_Fortran is treated exactly as a fortran file
  - Is consistent with f18's intent to not have any notion of preprocessed 
sources
  - Now the flang.f90 and flang.F90  tests are 
identical except for the filename

- Various minor whitespace changes and documentation fixes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang-not-installed.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran when in
+! --driver-mode=clang. This is legacy behaviour - see also --driver-mode=flang.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang --driver-mode=f

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2019-09-19 Thread Peter Waller via Phabricator via cfe-commits
peterwaller-arm added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:69
+CLMode,
+FlangMode,
   } Mode;

kiranchandramohan wrote:
> Is the comma by choice?
It was not. Fixed.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:37
+  if (isa(JA)) {
+CmdArgs.push_back("-emit-obj");
+  } else if (isa(JA)) {

kiranchandramohan wrote:
> peterwaller-arm wrote:
> > richard.barton.arm wrote:
> > > F18 does not currently support these options that control the output like 
> > > -emit-llvm and -emit-obj so this code doesn't do anything sensible at 
> > > present. Would it not make more sense to add this later on once F18 or 
> > > llvm/flang grows support for such options?
> > I've removed them.
> Can it be removed from the Summary of this PR?
They have now been reintroduced after we found that it had an unpleasant 
failure mode. Better to implement them now, after all, and fail gracefully in 
the frontend.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:44
+if (JA.getType() == types::TY_Nothing) {
+  CmdArgs.push_back("-fsyntax-only");
+} else if (JA.getType() == types::TY_LLVM_IR ||

richard.barton.arm wrote:
> peterwaller-arm wrote:
> > richard.barton.arm wrote:
> > > Looks like the F18 option spelling for this is -fparse-only? Or maybe 
> > > -fdebug-semantics? I know the intention is to throw away the 'throwaway 
> > > driver' in F18, so perhaps you are preparing to replace this option 
> > > spelling in the throwaway driver with -fsyntax-only so this would 
> > > highlight that perhaps adding the code here before the F18 driver is 
> > > ready to accept it is not the right approach?
> > In the RFC, the intent expressed was to mimick gfortran and clang options. 
> > I am making a spelling choice here that I think it should be called 
> > -fsyntax-only, which matches those. I intend to make the `flang -fc1` side 
> > match in the near future.
> > 
> > -fdebug-* could be supported through an `-Xflang ...` option to pass debug 
> > flags through.
> OK - happy with this approach. So -fsyntax-only and -emit-ast will be the new 
> names for f18's -fdebug-semantics and -fdebug-dump-parse-tree I guess.
Agreed. They can still be changed later if necessary.



Comment at: clang/lib/Driver/Types.cpp:220
+
+  case TY_Fortran: case TY_PP_Fortran:
+return true;

kiranchandramohan wrote:
> Now that there is a 2018 standard, I am assuming that f18 and F18 are also 
> valid fortran extensions. Can these be added to the File types?
> 
> Also, should the TypeInfo file be extended to handle all Fortran file types? 
> ./include/clang/Driver/Types.def
> 
> Also, should we capture some information about the standards from the 
> filename extension?
My reading of the code is that both [fF]90 and [fF]95 both get turned into 
TY_Fortran/TY_PP_Fortran.

Both of these in the Types.def end up coerced to "f95", currently. I don't 
think the driver wants an entry in the Types.def for each fortran standard - it 
doesn't have them for other languages, for example, and I'm unsure what the 
benefit of that would be. The main purpose of that table as far as I see is to 
determine what phases need to run.

My feeling is that the name in the types table should be "fortran". I don't 
want to make that change in the scope of this PR, since it might be a breaking 
change. It looks to me like the name works its way out towards human eyeballs 
but is otherwise unused.

I would like to implement -std inference from the file extension and additional 
fortran filename extensions (f18 etc) in another change request.



Comment at: clang/lib/Driver/Types.cpp:220
+
+  case TY_Fortran: case TY_PP_Fortran:
+return true;

richard.barton.arm wrote:
> peterwaller-arm wrote:
> > kiranchandramohan wrote:
> > > Now that there is a 2018 standard, I am assuming that f18 and F18 are 
> > > also valid fortran extensions. Can these be added to the File types?
> > > 
> > > Also, should the TypeInfo file be extended to handle all Fortran file 
> > > types? ./include/clang/Driver/Types.def
> > > 
> > > Also, should we capture some information about the standards from the 
> > > filename extension?
> > My reading of the code is that both [fF]90 and [fF]95 both get turned into 
> > TY_Fortran/TY_PP_Fortran.
> > 
> > Both of these in the Types.def end up coerced to "f95", currently. I don't 
> > think the driver wants an entry in the Types.def for each fortran standard 
> > - it doesn't have them for other languages, for example, and I'm unsure 
> > what the benefit of that would be. The main purpose of that table as far as 
> > I see is to determine what phases need to run.
> > 
> > My feeling is that the name in the types table should be "fortran". I don't 
> > want to make that change in the scope of this PR, since it might be a 
> > breaking change. It looks 

r372319 - Clean out unused diagnostics. NFC.

2019-09-19 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Thu Sep 19 06:35:27 2019
New Revision: 372319

URL: http://llvm.org/viewvc/llvm-project?rev=372319&view=rev
Log:
Clean out unused diagnostics. NFC.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=372319&r1=372318&r2=372319&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Thu Sep 19 06:35:27 2019
@@ -283,7 +283,6 @@ def warn_odr_variable_multiple_def : War
   "external variable %0 defined in multiple translation units">,
   InGroup;
 def note_odr_value_here : Note<"declared here with type %0">;
-def note_odr_defined_here : Note<"also defined here">;
 def err_odr_function_type_inconsistent : Error<
   "external function %0 declared with incompatible types in different "
   "translation units (%1 vs. %2)">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=372319&r1=372318&r2=372319&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Thu Sep 19 06:35:27 
2019
@@ -270,8 +270,6 @@ def err_target_unsupported_mcmse : Error
   "-mcmse is not supported for %0">;
 def err_opt_not_valid_with_opt : Error<
   "option '%0' cannot be specified with '%1'">;
-def err_opt_not_valid_without_opt : Error<
-  "option '%0' cannot be specified without '%1'">;
 def err_opt_not_valid_on_target : Error<
   "option '%0' cannot be specified on this target">;
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=372319&r1=372318&r2=372319&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Sep 19 06:35:27 
2019
@@ -182,9 +182,6 @@ def warn_drv_unknown_argument_clang_cl_w
 def warn_drv_ycyu_different_arg_clang_cl : Warning<
   "support for '/Yc' and '/Yu' with different filenames not implemented yet; 
flags ignored">,
   InGroup;
-def warn_drv_ycyu_no_fi_arg_clang_cl : Warning<
-  "support for '%0' without a corresponding /FI flag not implemented yet; flag 
ignored">,
-  InGroup;
 def warn_drv_yc_multiple_inputs_clang_cl : Warning<
   "support for '/Yc' with more than one source file not implemented yet; flag 
ignored">,
   InGroup;

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=372319&r1=372318&r2=372319&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Thu Sep 19 
06:35:27 2019
@@ -64,8 +64,6 @@ def err_fe_backend_unsupported : Error<"
 
 def err_fe_invalid_code_complete_file : Error<
 "cannot locate code-completion file %0">, DefaultFatal;
-def err_fe_stdout_binary : Error<"unable to change standard output to binary">,
-  DefaultFatal;
 def err_fe_dependency_file_requires_MT : Error<
 "-dependency-file requires at least one -MT or -MQ option">;
 def err_fe_invalid_plugin_name : Error<
@@ -217,10 +215,6 @@ def err_modules_embed_file_not_found :
   DefaultFatal;
 def err_module_header_file_not_found :
   Error<"module header file '%0' not found">, DefaultFatal;
-def err_module_header_file_invalid :
-  Error<"unexpected module header file input '%0'">, DefaultFatal;
-
-def err_interface_stubs : Error<"clang-ifs (-emit-iterface-stubs): %0">;
 
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected 
"

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=372319&r1=372318&r2=372319&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 19 06:35:27 
2019
@@ -2469,8 +2469,6 @@ def warn_cxx11_compat_constexpr_body_mul
   InGroup, DefaultIgnore;
 def note_

r372321 - [OpenCL] Add version handling and add vector ld/st builtins

2019-09-19 Thread Sven van Haastregt via cfe-commits
Author: svenvh
Date: Thu Sep 19 06:41:51 2019
New Revision: 372321

URL: http://llvm.org/viewvc/llvm-project?rev=372321&view=rev
Log:
[OpenCL] Add version handling and add vector ld/st builtins

Allow setting a MinVersion, stating from which OpenCL version a
builtin function is available, and a MaxVersion, stating from which
OpenCL version a builtin function should not be available anymore.

Guard some definitions of the "work-item" builtin functions according
to the OpenCL versions from which they are available.

Add the "vector data load and store" builtin functions (e.g.
vload/vstore), whose signatures differ before and after OpenCL 2.0 in
the pointer argument address spaces.

Patch by Pierre Gondois and Sven van Haastregt.

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

Modified:
cfe/trunk/lib/Sema/OpenCLBuiltins.td
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Modified: cfe/trunk/lib/Sema/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/OpenCLBuiltins.td?rev=372321&r1=372320&r2=372321&view=diff
==
--- cfe/trunk/lib/Sema/OpenCLBuiltins.td (original)
+++ cfe/trunk/lib/Sema/OpenCLBuiltins.td Thu Sep 19 06:41:51 2019
@@ -20,12 +20,13 @@
 
//===--===//
 // Versions of OpenCL
 class Version {
-  int Version = _Version;
+  int ID = _Version;
 }
-def CL10: Version<100>;
-def CL11: Version<110>;
-def CL12: Version<120>;
-def CL20: Version<200>;
+def CLAll : Version<  0>;
+def CL10  : Version<100>;
+def CL11  : Version<110>;
+def CL12  : Version<120>;
+def CL20  : Version<200>;
 
 // Address spaces
 // Pointer types need to be assigned an address space.
@@ -191,8 +192,13 @@ class Builtin _
   list Signature = _Signature;
   // OpenCL Extension to which the function belongs (cl_khr_subgroups, ...)
   string Extension = "";
-  // OpenCL Version to which the function belongs (CL10, ...)
-  Version Version = CL10;
+  // Version of OpenCL from which the function is available (e.g.: CL10).
+  // MinVersion is inclusive.
+  Version MinVersion = CL10;
+  // Version of OpenCL from which the function is not supported anymore.
+  // MaxVersion is exclusive.
+  // CLAll makes the function available for all versions.
+  Version MaxVersion = CLAll;
 }
 
 
//===--===//
@@ -313,6 +319,137 @@ foreach RType = [Float, Double, Half, Ch
 }
 
 //
+// OpenCL v1.1 s6.11.1, v1.2 s6.12.1, v2.0 s6.13.1 - Work-item Functions
+// --- Table 7 ---
+def : Builtin<"get_work_dim", [UInt]>;
+foreach name = ["get_global_size", "get_global_id", "get_local_size",
+"get_local_id", "get_num_groups", "get_group_id",
+"get_global_offset"] in {
+  def : Builtin;
+}
+
+let MinVersion = CL20 in {
+  def : Builtin<"get_enqueued_local_size", [Size, UInt]>;
+  foreach name = ["get_global_linear_id", "get_local_linear_id"] in {
+def : Builtin;
+  }
+}
+
+//
+// OpenCL v1.1 s6.11.7, v1.2 s6.12.7, v2.0 s6.13.7 - Vector Data Load and 
Store Functions
+// OpenCL Extension v1.1 s9.3.6 and s9.6.6, v1.2 s9.5.6, v2.0 s9.4.6, v2.0 
s5.1.6 and 6.1.6 - Vector Data Load and Store Functions
+// --- Table 15 ---
+// Variants for OpenCL versions below 2.0, using pointers to the global, local
+// and private address spaces.
+let MaxVersion = CL20 in {
+  foreach AS = [GlobalAS, LocalAS, PrivateAS] in {
+foreach VSize = [2, 3, 4, 8, 16] in {
+  foreach name = ["vload" # VSize] in {
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+  }
+  foreach name = ["vstore" # VSize] in {
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType, AS>]>;
+

[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-19 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added a comment.

Thanks for the changes @kmclaughlin! Just a few more nits from me, but looks 
good otherwise.




Comment at: include/llvm/IR/DerivedTypes.h:493
+llvm_unreachable("Cannot create narrower fp vector element type");
+break;
+  }

nit: I don't think the break is needed because of the `llvm_unreachable`



Comment at: lib/IR/Function.cpp:992
+assert(VTy && "Expected an argument of Vector Type");
+if (D.Kind == IITDescriptor::Subdivide2Argument)
+  return VectorType::getSubdividedVectorType(VTy, 1);

nit: you can simplify this by:
```
int SubDiv = D.Kind == IITDescriptor::Subdivide2Argument ? 1 : 2;
return VectorType::getSubdividedVectorType(VTy, SubDiv);
```



Comment at: lib/IR/Function.cpp:1302
+  Type *NewTy = ArgTys[D.getArgumentNumber()];
+  if (VectorType *VTy = dyn_cast(NewTy)) {
+if (D.Kind == IITDescriptor::Subdivide2Argument)

nit: you can use `auto *VTy` here since the `dyn_cast` makes it 
clear that the return type is `VectorType`.



Comment at: lib/IR/Function.cpp:1303
+  if (VectorType *VTy = dyn_cast(NewTy)) {
+if (D.Kind == IITDescriptor::Subdivide2Argument)
+  NewTy = VectorType::getSubdividedVectorType(VTy, 1);

nit: can be simplified by:
```int SubDiv = D.Kind == IITDescriptor::Subdivide2Argument ? 1 : 2;
NewTy = VectorType::getSubdividedVectorType(VTy, SubDiv);```



Comment at: lib/IR/Function.cpp:1310
+
+  return Ty != NewTy;
+}

nit: Move this return statement into the `if(VectorType *VTy = 
dyn_cast(NewTy))` block?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67549/new/

https://reviews.llvm.org/D67549



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


r372323 - Remove an unsafe member variable that wasn't needed; NFC.

2019-09-19 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Sep 19 06:51:50 2019
New Revision: 372323

URL: http://llvm.org/viewvc/llvm-project?rev=372323&view=rev
Log:
Remove an unsafe member variable that wasn't needed; NFC.

People use the AST dumping interface while debugging, so it's not safe to 
assume that a declaration will be dumped before a constant expression is 
dumped. This means the Context member may not get set properly and problems 
would happen. Rather than rely on the interface that requires the ASTContext, 
call the generic dump() interface instead; this allows us to remove the Context 
member variable.

Modified:
cfe/trunk/include/clang/AST/TextNodeDumper.h
cfe/trunk/lib/AST/TextNodeDumper.cpp

Modified: cfe/trunk/include/clang/AST/TextNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TextNodeDumper.h?rev=372323&r1=372322&r2=372323&view=diff
==
--- cfe/trunk/include/clang/AST/TextNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/TextNodeDumper.h Thu Sep 19 06:51:50 2019
@@ -146,8 +146,6 @@ class TextNodeDumper
 
   const comments::CommandTraits *Traits;
 
-  const ASTContext *Context;
-
   const char *getCommandName(unsigned CommandID);
 
 public:

Modified: cfe/trunk/lib/AST/TextNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TextNodeDumper.cpp?rev=372323&r1=372322&r2=372323&view=diff
==
--- cfe/trunk/lib/AST/TextNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp Thu Sep 19 06:51:50 2019
@@ -223,7 +223,6 @@ void TextNodeDumper::Visit(const Decl *D
 return;
   }
 
-  Context = &D->getASTContext();
   {
 ColorScope Color(OS, ShowColors, DeclKindNameColor);
 OS << D->getDeclKindName() << "Decl";
@@ -688,7 +687,7 @@ void TextNodeDumper::VisitConstantExpr(c
   if (Node->getResultAPValueKind() != APValue::None) {
 ColorScope Color(OS, ShowColors, ValueColor);
 OS << " ";
-Node->getAPValueResult().printPretty(OS, *Context, Node->getType());
+Node->getAPValueResult().dump(OS);
   }
 }
 


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


r372325 - Reverting r372323 because it broke color tests on Linux.

2019-09-19 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Sep 19 06:59:53 2019
New Revision: 372325

URL: http://llvm.org/viewvc/llvm-project?rev=372325&view=rev
Log:
Reverting r372323 because it broke color tests on Linux.

http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/17919

Modified:
cfe/trunk/include/clang/AST/TextNodeDumper.h
cfe/trunk/lib/AST/TextNodeDumper.cpp

Modified: cfe/trunk/include/clang/AST/TextNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TextNodeDumper.h?rev=372325&r1=372324&r2=372325&view=diff
==
--- cfe/trunk/include/clang/AST/TextNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/TextNodeDumper.h Thu Sep 19 06:59:53 2019
@@ -146,6 +146,8 @@ class TextNodeDumper
 
   const comments::CommandTraits *Traits;
 
+  const ASTContext *Context;
+
   const char *getCommandName(unsigned CommandID);
 
 public:

Modified: cfe/trunk/lib/AST/TextNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TextNodeDumper.cpp?rev=372325&r1=372324&r2=372325&view=diff
==
--- cfe/trunk/lib/AST/TextNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp Thu Sep 19 06:59:53 2019
@@ -223,6 +223,7 @@ void TextNodeDumper::Visit(const Decl *D
 return;
   }
 
+  Context = &D->getASTContext();
   {
 ColorScope Color(OS, ShowColors, DeclKindNameColor);
 OS << D->getDeclKindName() << "Decl";
@@ -687,7 +688,7 @@ void TextNodeDumper::VisitConstantExpr(c
   if (Node->getResultAPValueKind() != APValue::None) {
 ColorScope Color(OS, ShowColors, ValueColor);
 OS << " ";
-Node->getAPValueResult().dump(OS);
+Node->getAPValueResult().printPretty(OS, *Context, Node->getType());
   }
 }
 


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


[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

2019-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/APValue.h:512
   }
-  void setVector(const APValue *E, unsigned N) {
+  void ReserveVector(unsigned N) {
 assert(isVector() && "Invalid accessor");

`reserveVector` per naming conventions



Comment at: clang/include/clang/AST/ASTContext.h:275
-  /// Used to cleanups APValues stored in the AST.
-  mutable llvm::SmallVector APValueCleanups;
-

Why are you getting rid of this? It seems like we would still want these 
cleaned up.



Comment at: clang/include/clang/AST/Expr.h:957
   /// Describes the kind of result that can be trail-allocated.
+  /// Enumerator need to stay ordered by size of there storage.
   enum ResultStorageKind { RSK_None, RSK_Int64, RSK_APValue };

Enumerator -> Enumerators
there -> their



Comment at: clang/include/clang/AST/TextNodeDumper.h:149
 
-  const ASTContext *Context;
+  const ASTContext *Context = nullptr;
 

Good catch -- this pointed out a bug in the class that I've fixed in r372323, 
so you'll need to rebase.



Comment at: clang/lib/AST/APValue.cpp:176
+  (DataSize - sizeof(MemberPointerBase)) / sizeof(CXXRecordDecl *);
+  typedef CXXRecordDecl *PathElem;
   union {

Why is this no longer a pointer to `const`?



Comment at: clang/lib/AST/APValue.cpp:748
 
+APValue::LValuePathEntry *APValue::getLValuePathPtr() {
+  return ((LV *)(char *)Data.buffer)->getPath();

Can this function be marked `const`?



Comment at: clang/lib/AST/ASTImporter.cpp:8745
 
+<<< HEAD
 Expected ASTImporter::HandleNameConflict(DeclarationName Name,

Conflict markers?



Comment at: clang/lib/AST/ASTImporter.cpp:8808
+Result.MakeUnion();
+auto ImpFDecl = Import(FromValue.getUnionField());
+if (!ImpFDecl) {

Please don't use auto as the type is not spelled out in the initialization.



Comment at: clang/lib/AST/ASTImporter.cpp:8813
+}
+auto ImpValue = Import(FromValue.getUnionValue());
+if (!ImpValue) {

Please don't use `auto` as the type is not spelled out in the initialization.

(Same elsewhere, I'll stop commenting on them.)



Comment at: clang/lib/AST/TextNodeDumper.cpp:691-695
+if (Context)
+  Node->getAPValueResult().printPretty(OS, *Context, Node->getType());
+else
+  Node->getAPValueResult().dump(OS);
+  } else {}

You'll have to rebase this too -- these changes shouldn't be needed any longer.



Comment at: clang/lib/Sema/SemaDecl.cpp:11315
 
+  if (VDecl->isConstexpr()) {
+Init = ConstantExpr::Create(

Can elide braces.



Comment at: clang/lib/Serialization/ASTReader.cpp:9601
+  case APValue::Union: {
+FieldDecl *FDecl =
+cast(GetDecl(ReadDeclID(F, Record, RecordIdx)));

`auto *`



Comment at: clang/lib/Serialization/ASTReader.cpp:9607-9608
+  case APValue::AddrLabelDiff: {
+AddrLabelExpr *LHS = cast(ReadExpr(F));
+AddrLabelExpr *RHS = cast(ReadExpr(F));
+return APValue(LHS, RHS);

`auto *`



Comment at: clang/lib/Serialization/ASTReader.cpp:9614
+bool IsDerived = Record[RecordIdx++];
+ValueDecl *Member =
+cast(GetDecl(ReadDeclID(F, Record, RecordIdx)));

`auto *`



Comment at: clang/lib/Serialization/ASTReader.cpp:9625-9631
+uint64_t tmp = Record[RecordIdx++];
+bool hasLValuePath = tmp & 0x1;
+bool isLValueOnePastTheEnd = tmp & 0x2;
+bool isExpr = tmp & 0x4;
+bool isTypeInfo = tmp & 0x8;
+bool isNullPtr = tmp & 0x10;
+bool hasBase = tmp & 0x20;

These names don't match the current coding conventions.



Comment at: clang/lib/Serialization/ASTReader.cpp:9634
+QualType ElemTy;
+assert(!isExpr || !isTypeInfo && "LValueBase cannot be both");
+if (hasBase) {

You should add parens here to avoid diagnostics.



Comment at: clang/lib/Serialization/ASTWriter.cpp:5485
+  case APValue::LValue: {
+uint64_t tmp = Value.hasLValuePath() | Value.isLValueOnePastTheEnd() << 1 |
+   Value.getLValueBase().is() << 2 |

Name doesn't match coding conventions.



Comment at: clang/lib/Serialization/ASTWriter.cpp:5496
+push_back(Value.getLValueBase().getVersion());
+if (auto *E = Value.getLValueBase().dyn_cast()) {
+  AddStmt(const_cast(E));

`const auto *`



Comment at: clang/lib/Serialization/ASTWriter.cpp:5518
+  const Decl *BaseOrMember = Elem.getAsBaseOrMember().getPointer();
+  if (const CXXRecordDecl *RD = dyn_cast(BaseOrMember)) 
{
+AddDec

[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

2019-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp:56
+  const auto *ID = Result.Nodes.getNodeAs("impl");
+  diag(ID->getLocation(), "%0 implements -isEqual: without implementing -hash")
+  << ID;

Do you think we could generate a fixit to add the `hash` method? Do you think 
we could even add a default implementation that returns the pointer to the 
object (assuming that's the correct default behavior)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67737/new/

https://reviews.llvm.org/D67737



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


[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2019-09-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done.
Szelethus added inline comments.
Herald added a subscriber: Charusso.
Herald added a project: LLVM.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3090
   return checkPointerEscapeAux(State, Escaped, Call, Kind,
-   &checkIfNewOrNewArrayFamily);
+   /*IsConstPointerEscape*/ true);
 }

Praise clangd! This is why the buildbots broke :) `checkIfNewOrNewArrayFamily` 
doesn't return `false` in all cases.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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


r372334 - Revert r372325 - Reverting r372323 because it broke color tests on Linux.

2019-09-19 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Sep 19 08:10:51 2019
New Revision: 372334

URL: http://llvm.org/viewvc/llvm-project?rev=372334&view=rev
Log:
Revert r372325 - Reverting r372323 because it broke color tests on Linux.

This corrects the testing issues.

Modified:
cfe/trunk/include/clang/AST/TextNodeDumper.h
cfe/trunk/lib/AST/TextNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-color.cpp

Modified: cfe/trunk/include/clang/AST/TextNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TextNodeDumper.h?rev=372334&r1=372333&r2=372334&view=diff
==
--- cfe/trunk/include/clang/AST/TextNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/TextNodeDumper.h Thu Sep 19 08:10:51 2019
@@ -146,8 +146,6 @@ class TextNodeDumper
 
   const comments::CommandTraits *Traits;
 
-  const ASTContext *Context;
-
   const char *getCommandName(unsigned CommandID);
 
 public:

Modified: cfe/trunk/lib/AST/TextNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TextNodeDumper.cpp?rev=372334&r1=372333&r2=372334&view=diff
==
--- cfe/trunk/lib/AST/TextNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp Thu Sep 19 08:10:51 2019
@@ -223,7 +223,6 @@ void TextNodeDumper::Visit(const Decl *D
 return;
   }
 
-  Context = &D->getASTContext();
   {
 ColorScope Color(OS, ShowColors, DeclKindNameColor);
 OS << D->getDeclKindName() << "Decl";
@@ -688,7 +687,7 @@ void TextNodeDumper::VisitConstantExpr(c
   if (Node->getResultAPValueKind() != APValue::None) {
 ColorScope Color(OS, ShowColors, ValueColor);
 OS << " ";
-Node->getAPValueResult().printPretty(OS, *Context, Node->getType());
+Node->getAPValueResult().dump(OS);
   }
 }
 

Modified: cfe/trunk/test/AST/ast-dump-color.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-color.cpp?rev=372334&r1=372333&r2=372334&view=diff
==
--- cfe/trunk/test/AST/ast-dump-color.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-color.cpp Thu Sep 19 08:10:51 2019
@@ -49,13 +49,13 @@ struct Invalid {
 //CHECK: {{^}}[[Blue]]| |   
|-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:11[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan:.\[0;36m]][[RESET]][[Cyan]][[RESET]][[CYAN]] 
1[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| |   
`-[[RESET]][[MAGENTA]]CompoundStmt[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:14[[RESET]], 
[[Yellow]]line:15:3[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | 
|-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] 
<[[Yellow]]line:11:3[[RESET]], [[Yellow]]line:12:27[[RESET]]>{{$}}
-//CHECK: {{^}}[[Blue]]| | | 
|-[[RESET]][[MAGENTA]]ConstantExpr[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 
1[[RESET]]{{$}}
+//CHECK: {{^}}[[Blue]]| | | 
|-[[RESET]][[MAGENTA]]ConstantExpr[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] Int: 
1[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| | | | 
`-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 
1[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| | | 
`-[[RESET]][[MAGENTA]]AttributedStmt[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:12:5[[RESET]], 
[[Yellow]]col:27[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | |   
|-[[RESET]][[BLUE]]FallThroughAttr[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:7[[RESET]], 
[[Yellow]]col:14[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | |   
`-[[RESET]][[MAGENTA]]NullStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] 
<[[Yellow]]col:27[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | 
`-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] 
<[[Yellow]]line:13:3[[RESET]], [[Yellow]]line:14:5[[RESET]]>{{$}}
-//CHECK: {{^}}[[Blue]]| |   
|-[[RESET]][[MAGENTA]]ConstantExpr[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:13:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 
2[[RESET]]{{$}}
+//CHECK: {{^}}[[Blue]]| |   
|-[[RESET]][[MAGENTA]]ConstantExpr[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:13:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] Int: 
2[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| |   | 
`-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:8[[RESET]]> 
[[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 
2[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| |   
`-[[RESET]]

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-09-19 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment.

We discussed this in the RISC-V meeting on 19 Sept 2019. @apazos says there are 
some SPEC failures in both 2006 and 2017, which would be good to triage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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


[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-09-19 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment.

Two nits, that I wanted to submit before the meeting, but didn't get around to.




Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;

Given the clang option defaults to false, I think it should here too, to avoid 
confusion in other frontends.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:278
   // Add CFI directives for callee-saved registers.
-  const std::vector &CSI = MFI.getCalleeSavedInfo();
-  // Iterate over list of callee-saved registers and emit .cfi_restore
-  // directives.
-  for (const auto &Entry : CSI) {
-Register Reg = Entry.getReg();
-unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
-nullptr, RI->getDwarfRegNum(Reg, true)));
-BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
-.addCFIIndex(CFIIndex);
+  if (!CSI.empty()) {
+// Iterate over list of callee-saved registers and emit .cfi_restore

Nit: You shouldn't need this `if` statement - the for loop just won't execute 
if CSI is empty, surely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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


[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2019-09-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 220873.
Szelethus added a comment.

Rebase, fix (suspected) error that caused buildbot errors. Hold off commiting 
in favor checking whether putting `CallDescriptionMap` in would be too 
invasive, but really, can't be worse then it already is.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -6,8 +6,41 @@
 //
 //===--===//
 //
-// This file defines malloc/free checker, which checks for potential memory
-// leaks, double free, and use-after-free problems.
+// This file defines a variety of memory management related checkers, such as
+// leak, double free, and use-after-free.
+//
+// The following checkers are defined here:
+//
+//   * MallocChecker
+//   Despite its name, it models all sorts of memory allocations and
+//   de- or reallocation, including but not limited to malloc, free,
+//   relloc, new, delete. It also reports on a variety of memory misuse
+//   errors.
+//   Many other checkers interact very closely with this checker, in fact,
+//   most are merely options to this one. Other checkers may register
+//   MallocChecker, but do not enable MallocChecker's reports (more details
+//   to follow around its field, ChecksEnabled).
+//   It also has a boolean "Optimistic" checker option, which if set to true
+//   will cause the checker to model user defined memory management related
+//   functions annotated via the attribute ownership_takes, ownership_holds
+//   and ownership_returns.
+//
+//   * NewDeleteChecker
+//   Enables the modeling of new, new[], delete, delete[] in MallocChecker,
+//   and checks for related double-free and use-after-free errors.
+//
+//   * NewDeleteLeaksChecker
+//   Checks for leaks related to new, new[], delete, delete[].
+//   Depends on NewDeleteChecker.
+//
+//   * MismatchedDeallocatorChecker
+//   Enables checking whether memory is deallocated with the correspending
+//   allocation function in MallocChecker, such as malloc() allocated
+//   regions are only freed by free(), new by delete, new[] by delete[].
+//
+//  InnerPointerChecker interacts very closely with MallocChecker, but unlike
+//  the above checkers, it has it's own file, hence the many InnerPointerChecker
+//  related headers and non-static functions.
 //
 //===--===//
 
@@ -37,6 +70,10 @@
 using namespace clang;
 using namespace ento;
 
+//===--===//
+// The types of allocation we're modeling.
+//===--===//
+
 namespace {
 
 // Used to check correspondence between allocators and deallocators.
@@ -50,57 +87,88 @@
   AF_InnerBuffer
 };
 
+struct MemFunctionInfoTy;
+
+} // end of anonymous namespace
+
+/// Determine family of a deallocation expression.
+static AllocationFamily
+getAllocationFamily(const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C,
+const Stmt *S);
+
+/// Print names of allocators and deallocators.
+///
+/// \returns true on success.
+static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
+  const Expr *E);
+
+/// Print expected name of an allocator based on the deallocator's
+/// family derived from the DeallocExpr.
+static void printExpectedAllocName(raw_ostream &os,
+   const MemFunctionInfoTy &MemFunctionInfo,
+   CheckerContext &C, const Expr *E);
+
+/// Print expected name of a deallocator based on the allocator's
+/// family.
+static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family);
+
+//===--===//
+// The state of a symbol, in terms of memory management.
+//===--===//
+
+namespace {
+
 class RefState {
-  enum Kind { // Reference to allocated memory.
-  Allocated,
-  // Reference to zero-allocated memory.
-  AllocatedOfSizeZero,
-  // Reference to released/freed memory.
-  Released,
-  // The responsibility for freeing resources has transferred from
-  // this reference. A relinquished symbol should not be freed.
-  Relinquished,
-  // We are no longer guaranteed to have observed all manipulations
-  // of this pointer/memory. For exa

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

2019-09-19 Thread Simon Cook via Phabricator via cfe-commits
simoncook updated this revision to Diff 220876.
simoncook added a comment.

Update to reflect comments about the fact registers are explicitly reserved. In 
addition to @lenary 's suggested change, I renamed `isReservedReg` to note the 
check that we are checking if its a user provided reservation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67185/new/

https://reviews.llvm.org/D67185

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-fixed-x-register.c
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.h
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/lib/Target/RISCV/RISCVRegisterInfo.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/reserved-reg-errors.ll
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -0,0 +1,130 @@
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x3 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X3
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x3 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X3
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x4 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X4
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x4 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X4
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x5 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X5
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x5 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X5
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x6 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X6
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x6 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X6
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x7 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X7
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x7 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X7
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x8 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X8
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x8 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X8
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x9 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X9
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x9 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X9
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x10 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X10
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x10 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X10
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x11 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X11
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x11 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X11
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x12 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X12
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x12 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X12
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x13 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X13
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x13 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X13
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x14 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X14
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x14 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X14
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x15 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X15
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x15 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X15
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x16 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X16
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x16 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X16
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x17 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X17
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x17 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X17
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x18 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x18 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x19 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X19
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x19 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X19
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x20 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X20
+; RU

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-09-19 Thread Lewis Revill via Phabricator via cfe-commits
lewis-revill marked 2 inline comments as done.
lewis-revill added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;

lenary wrote:
> Given the clang option defaults to false, I think it should here too, to 
> avoid confusion in other frontends.
This is simply a case of a confusing parameter of the SubtargetFeature class.

SubtargetFeature interprets this "true" string as 'Value the attribute to be 
set to by feature'. IE: when the feature is enabled, what value should the 
corresponding RISCVSubtarget variable be set to? Rather than a default value 
for that variable.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:278
   // Add CFI directives for callee-saved registers.
-  const std::vector &CSI = MFI.getCalleeSavedInfo();
-  // Iterate over list of callee-saved registers and emit .cfi_restore
-  // directives.
-  for (const auto &Entry : CSI) {
-Register Reg = Entry.getReg();
-unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
-nullptr, RI->getDwarfRegNum(Reg, true)));
-BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
-.addCFIIndex(CFIIndex);
+  if (!CSI.empty()) {
+// Iterate over list of callee-saved registers and emit .cfi_restore

lenary wrote:
> Nit: You shouldn't need this `if` statement - the for loop just won't execute 
> if CSI is empty, surely.
Good catch, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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


[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-09-19 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:72
 
+def FeatureSaveRestore : SubtargetFeature<"save-restore", "EnableSaveRestore",
+  "true", "Enable save/restore.">;

lewis-revill wrote:
> lenary wrote:
> > Given the clang option defaults to false, I think it should here too, to 
> > avoid confusion in other frontends.
> This is simply a case of a confusing parameter of the SubtargetFeature class.
> 
> SubtargetFeature interprets this "true" string as 'Value the attribute to be 
> set to by feature'. IE: when the feature is enabled, what value should the 
> corresponding RISCVSubtarget variable be set to? Rather than a default value 
> for that variable.
Ah, apologies for the confusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Sema/SemaCUDA.cpp:273-274
+ MemberDecl->hasAttr();
+  if (!InClass || hasAttr)
+return false;
+

A comment here would be helpful.

I think the intent here is to look for implicit special members with 
*explicitly* set attributes.
We have number of cases where we set H/D attributes implicitly. I'm not sure 
whether we ever see any of them here, but if we do, it will sneak through this 
check. I think a check for whether the attribute is explicit would be prudent.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-09-19 Thread Lewis Revill via Phabricator via cfe-commits
lewis-revill marked an inline comment as done.
lewis-revill added a comment.

In D62686#1675347 , @lenary wrote:

> We discussed this in the RISC-V meeting on 19 Sept 2019. @apazos says there 
> are some SPEC failures in both 2006 and 2017, which would be good to triage.


I've uncovered some execution failures in our annotated GCC testsuite as well, 
so I will look into those as a starting point.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: lib/Sema/SemaCUDA.cpp:273-274
+ MemberDecl->hasAttr();
+  if (!InClass || hasAttr)
+return false;
+

tra wrote:
> A comment here would be helpful.
> 
> I think the intent here is to look for implicit special members with 
> *explicitly* set attributes.
> We have number of cases where we set H/D attributes implicitly. I'm not sure 
> whether we ever see any of them here, but if we do, it will sneak through 
> this check. I think a check for whether the attribute is explicit would be 
> prudent.
will add the comment.

I intentionally omitted check for explicit attr because I noticed the same 
special member is inferred twice. Each time it is added the same attrs, which 
cause them to have two `__host__` and two `__device__` attrs. By checking if 
attrs exist (not just explicit attrs) we can avoid duplicate attrs. I tested 
this with real machine learning frameworks and did not see issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Sema/SemaCUDA.cpp:273-274
+ MemberDecl->hasAttr();
+  if (!InClass || hasAttr)
+return false;
+

yaxunl wrote:
> tra wrote:
> > A comment here would be helpful.
> > 
> > I think the intent here is to look for implicit special members with 
> > *explicitly* set attributes.
> > We have number of cases where we set H/D attributes implicitly. I'm not 
> > sure whether we ever see any of them here, but if we do, it will sneak 
> > through this check. I think a check for whether the attribute is explicit 
> > would be prudent.
> will add the comment.
> 
> I intentionally omitted check for explicit attr because I noticed the same 
> special member is inferred twice. Each time it is added the same attrs, which 
> cause them to have two `__host__` and two `__device__` attrs. By checking if 
> attrs exist (not just explicit attrs) we can avoid duplicate attrs. I tested 
> this with real machine learning frameworks and did not see issues.
OK. So, if we have explicit attributes, then there's no need to infer them. If 
the attributes are implicit, then we've already guessed them, so there's no 
point doing it again. The check for simple attribute presence covers both 
cases. I'll buy that. 

This leaves a hypothetical gap in case we have implicitly set attributes set 
somewhere else that would disagree with the attributes that would be set by 
this function.  It would be great to have an assertion somewhere to verify that 
it does not happen. Alas, this function modifies the MemberDecl, so I don't see 
an easy way to do it.

In general, the multiple application of the attributes seems to be a separate 
issue that should be fixed. I think we run into it in other places, too. 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

2019-09-19 Thread Kerry McLaughlin via Phabricator via cfe-commits
kmclaughlin updated this revision to Diff 220886.
kmclaughlin added a comment.

- Some minor changes, including removing an unnecessary break
- Simplified checks of D.Kind in Function.cpp to determine if it is a 
Subdivide2Argument or Subdivide4Argument


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67549/new/

https://reviews.llvm.org/D67549

Files:
  include/llvm/IR/DerivedTypes.h
  include/llvm/IR/Intrinsics.h
  include/llvm/IR/Intrinsics.td
  lib/IR/Function.cpp
  utils/TableGen/IntrinsicEmitter.cpp

Index: utils/TableGen/IntrinsicEmitter.cpp
===
--- utils/TableGen/IntrinsicEmitter.cpp
+++ utils/TableGen/IntrinsicEmitter.cpp
@@ -221,7 +221,9 @@
   IIT_STRUCT8 = 40,
   IIT_F128 = 41,
   IIT_VEC_ELEMENT = 42,
-  IIT_SCALABLE_VEC = 43
+  IIT_SCALABLE_VEC = 43,
+  IIT_SUBDIVIDE2_ARG = 44,
+  IIT_SUBDIVIDE4_ARG = 45
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -293,6 +295,10 @@
   Sig.push_back(IIT_PTR_TO_ELT);
 else if (R->isSubClassOf("LLVMVectorElementType"))
   Sig.push_back(IIT_VEC_ELEMENT);
+else if (R->isSubClassOf("LLVMSubdivide2VectorType"))
+  Sig.push_back(IIT_SUBDIVIDE2_ARG);
+else if (R->isSubClassOf("LLVMSubdivide4VectorType"))
+  Sig.push_back(IIT_SUBDIVIDE4_ARG);
 else
   Sig.push_back(IIT_ARG);
 return Sig.push_back((Number << 3) | 7 /*IITDescriptor::AK_MatchType*/);
Index: lib/IR/Function.cpp
===
--- lib/IR/Function.cpp
+++ lib/IR/Function.cpp
@@ -703,7 +703,9 @@
   IIT_STRUCT8 = 40,
   IIT_F128 = 41,
   IIT_VEC_ELEMENT = 42,
-  IIT_SCALABLE_VEC = 43
+  IIT_SCALABLE_VEC = 43,
+  IIT_SUBDIVIDE2_ARG = 44,
+  IIT_SUBDIVIDE4_ARG = 45
 };
 
 static void DecodeIITType(unsigned &NextElt, ArrayRef Infos,
@@ -868,6 +870,18 @@
   DecodeIITType(NextElt, Infos, OutputTable);
 return;
   }
+  case IIT_SUBDIVIDE2_ARG: {
+unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
+OutputTable.push_back(IITDescriptor::get(IITDescriptor::Subdivide2Argument,
+ ArgInfo));
+return;
+  }
+  case IIT_SUBDIVIDE4_ARG: {
+unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
+OutputTable.push_back(IITDescriptor::get(IITDescriptor::Subdivide4Argument,
+ ArgInfo));
+return;
+  }
   case IIT_VEC_ELEMENT: {
 unsigned ArgInfo = (NextElt == Infos.size() ? 0 : Infos[NextElt++]);
 OutputTable.push_back(IITDescriptor::get(IITDescriptor::VecElementArgument,
@@ -970,6 +984,14 @@
 assert(ITy->getBitWidth() % 2 == 0);
 return IntegerType::get(Context, ITy->getBitWidth() / 2);
   }
+  case IITDescriptor::Subdivide2Argument:
+  case IITDescriptor::Subdivide4Argument: {
+Type *Ty = Tys[D.getArgumentNumber()];
+VectorType *VTy = dyn_cast(Ty);
+assert(VTy && "Expected an argument of Vector Type");
+int SubDivs = D.Kind == IITDescriptor::Subdivide2Argument ? 1 : 2;
+return VectorType::getSubdividedVectorType(VTy, SubDivs);
+  }
   case IITDescriptor::HalfVecArgument:
 return VectorType::getHalfElementsVectorType(cast(
   Tys[D.getArgumentNumber()]));
@@ -1269,6 +1291,20 @@
   auto *ReferenceType = dyn_cast(ArgTys[D.getArgumentNumber()]);
   return !ReferenceType || Ty != ReferenceType->getElementType();
 }
+case IITDescriptor::Subdivide2Argument:
+case IITDescriptor::Subdivide4Argument: {
+  // If this is a forward reference, defer the check for later.
+  if (D.getArgumentNumber() >= ArgTys.size())
+return IsDeferredCheck || DeferCheck(Ty);
+
+  Type *NewTy = ArgTys[D.getArgumentNumber()];
+  if (auto *VTy = dyn_cast(NewTy)) {
+int SubDivs = D.Kind == IITDescriptor::Subdivide2Argument ? 1 : 2;
+NewTy = VectorType::getSubdividedVectorType(VTy, SubDivs);
+return Ty != NewTy;
+  }
+  return true;
+}
 case IITDescriptor::ScalableVecArgument: {
   VectorType *VTy = dyn_cast(Ty);
   if (!VTy || !VTy->isScalable())
Index: include/llvm/IR/Intrinsics.td
===
--- include/llvm/IR/Intrinsics.td
+++ include/llvm/IR/Intrinsics.td
@@ -187,6 +187,12 @@
 // vector type, but change the element count to be half as many
 class LLVMHalfElementsVectorType : LLVMMatchType;
 
+// Match the type of another intrinsic parameter that is expected to be a
+// vector type (i.e. ) but with each element subdivided to
+// form a vector with more elements that are smaller than the original.
+class LLVMSubdivide2VectorType : LLVMMatchType;
+class LLVMSubdivide4VectorType : LLVMMatchType;
+
 def llvm_void_ty   : LLVMType;
 let isAny = 1 in {
   def llvm_any_ty: LLVMType;
Index: include/llvm/IR/Intrinsics.h
===
--- inc

[PATCH] D67635: Fix for stringized function-macro args continued across lines

2019-09-19 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, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67635/new/

https://reviews.llvm.org/D67635



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


[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

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

In D54823#1675352 , @Szelethus wrote:

> Rebase, fix (suspected) error that caused buildbot errors. Hold off commiting 
> in favor checking whether putting `CallDescriptionMap` in would be too 
> invasive, but really, can't be worse then it already is.


Nope. It implies far too heavy changes. I'll land this as is, but will wait a 
bit as its been a while since the acceptance.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 220892.
yaxunl edited the summary of this revision.
yaxunl added a comment.

Skip inferring for explicit host/device attrs only. Adds checks for implicit 
device and host attrs and avoid duplicates.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509

Files:
  lib/Sema/SemaCUDA.cpp
  test/SemaCUDA/default-ctor.cu

Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:-fcuda-is-device -verify -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only \
+// RUN:-verify -verify-ignore-unexpected=note %s
+
+#include "Inputs/cuda.h"
+
+struct In { In() = default; };
+struct InD { __device__ InD() = default; };
+struct InH { __host__ InH() = default; };
+struct InHD { __host__ __device__ InHD() = default; };
+
+struct Out { Out(); };
+struct OutD { __device__ OutD(); };
+struct OutH { __host__ OutH(); };
+struct OutHD { __host__ __device__ OutHD(); };
+
+Out::Out() = default;
+__device__ OutD::OutD() = default;
+__host__ OutH::OutH() = default;
+__host__ __device__ OutHD::OutHD() = default;
+
+__device__ void fd() {
+  In in;
+  InD ind;
+  InH inh; // expected-error{{no matching constructor for initialization of 'InH'}}
+  InHD inhd;
+  Out out; // expected-error{{no matching constructor for initialization of 'Out'}}
+  OutD outd;
+  OutH outh; // expected-error{{no matching constructor for initialization of 'OutH'}}
+  OutHD outhd;
+}
+
+__host__ void fh() {
+  In in;
+  InD ind; // expected-error{{no matching constructor for initialization of 'InD'}}
+  InH inh;
+  InHD inhd;
+  Out out;
+  OutD outd; // expected-error{{no matching constructor for initialization of 'OutD'}}
+  OutH outh;
+  OutHD outhd;
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -267,6 +267,17 @@
CXXMethodDecl *MemberDecl,
bool ConstRHS,
bool Diagnose) {
+  // If the defaulted special member is defined lexically outside of its
+  // owning class, or the special member already has explicit device or host
+  // attributes, do not infer.
+  bool InClass = MemberDecl->getLexicalParent() == MemberDecl->getParent();
+  bool HasExpAttr = (MemberDecl->hasAttr() &&
+ !MemberDecl->getAttr()->isImplicit()) ||
+(MemberDecl->hasAttr() &&
+ !MemberDecl->getAttr()->isImplicit());
+  if (!InClass || HasExpAttr)
+return false;
+
   llvm::Optional InferredTarget;
 
   // We're going to invoke special member lookup; mark that these special
@@ -371,20 +382,31 @@
 }
   }
 
+  // The same special member may be inferred multiple times. Check to make sure
+  // each inference gets same result and not to add duplicate attributes.
+  auto addBothAttr = [=]() {
+assert(MemberDecl->hasAttr() ==
+   MemberDecl->hasAttr());
+if (!MemberDecl->hasAttr())
+  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
+if (!MemberDecl->hasAttr())
+  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
+  };
   if (InferredTarget.hasValue()) {
 if (InferredTarget.getValue() == CFT_Device) {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
+  assert(!MemberDecl->hasAttr());
+  if (!MemberDecl->hasAttr())
+MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
 } else if (InferredTarget.getValue() == CFT_Host) {
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-} else {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-}
+  assert(!MemberDecl->hasAttr());
+  if (!MemberDecl->hasAttr())
+MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
+} else
+  addBothAttr();
   } else {
 // If no target was inferred, mark this member as __host__ __device__;
 // it's the least restrictive option that can be invoked from any target.
-MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
+addBothAttr();
   }
 
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Sema/SemaCUDA.cpp:387
+  // each inference gets same result and not to add duplicate attributes.
+  auto addBothAttr = [=]() {
+assert(MemberDecl->hasAttr() ==

`addHDAttrIfNeeded` ? We may not even need it. See below.



Comment at: lib/Sema/SemaCUDA.cpp:388-409
+assert(MemberDecl->hasAttr() ==
+   MemberDecl->hasAttr());
+if (!MemberDecl->hasAttr())
+  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
+if (!MemberDecl->hasAttr())
+  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
+  };

Perhaps we can rearrange things a bit to make it easier to follow.

```
bool needsH = true, needsD=true;
if (has Value) {
   if (CFT_Device)
  needsH = false;
   if (CFT_Host)
  needsD = false;
}

// We either setting attributes first time, or the inferred ones must match 
previously set ones.
assert(!(hasAttr(D) || hasAttr(H)) 
|| (needsD == hasAttr(D) && (needsH == hasAttr(H)))
if (needsD && !hasAttr(D))
   addAttr(D);
if (needsH && ! hasAttr(H))
   addAttr(H);
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


[PATCH] D67743: [Consumed] Treat by-value class arguments as consuming by default, like rvalue refs.

2019-09-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

"Also, fix the order of if statements so that an explicit return_typestate 
annotation takes precedence over the default behavior for rvalue refs."

I'd probably have split that out into a separate patch - in part to discuss the 
design implications of that. I have some doubts about supporting non-consumed 
after passing by rvalue ref, but don't feel too strongly & the alternative 
would be having a warning about the attribute being ignored, etc - when it has 
a fairly clear meaning if it is there, just not sure people 'should' be doing 
that.




Comment at: test/SemaCXX/warn-consumed-analysis.cpp:62-64
+
+  static void byVal(DestructorTester);
+  static void byValMarkUnconsumed(DestructorTester 
RETURN_TYPESTATE(unconsumed));

unless these are testing something noteworthy about them being static functions 
I'd probably suggest making them non-members like the other test functions 
below, for consistency (otherwise the inconsistency tends to raise the question 
of "what is significant about this difference?")


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67743/new/

https://reviews.llvm.org/D67743



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


[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

2019-09-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, owenpan.
MyDeveloperDay added a project: clang-tools-extra.
Herald added a project: clang.

This is to address a request made in https://bugs.llvm.org/show_bug.cgi?id=43144

Add the ability to not have a space before and after brace (limited to 
if,else,for,while,do keyword (for now))

  if (x){
  }else if{
  }else{
  }



  do{
  }while(x)



  while(x){
  }



  for(int i=0;ihttps://github.com/s3fs-fuse/s3fs-fuse) but noticed on reading this request 
they don't have a .clang-format file in their Github repo. Perhaps this is 
because without this feature we can't support their style at all.  My aim here 
is to further the reach of clang-format whilst increasing my knowledge of the 
workings for clang-format.


Repository:
  rC Clang

https://reviews.llvm.org/D67773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1596,7 +1596,7 @@
 TEST_F(FormatTest, BreakInheritanceStyle) {
   FormatStyle StyleWithInheritanceBreakBeforeComma = getLLVMStyle();
   StyleWithInheritanceBreakBeforeComma.BreakInheritanceList =
-  FormatStyle::BILS_BeforeComma;
+  FormatStyle::BILS_BeforeComma;
   verifyFormat("class MyClass : public X {};",
StyleWithInheritanceBreakBeforeComma);
   verifyFormat("class MyClass\n"
@@ -1614,7 +1614,7 @@
 
   FormatStyle StyleWithInheritanceBreakAfterColon = getLLVMStyle();
   StyleWithInheritanceBreakAfterColon.BreakInheritanceList =
-  FormatStyle::BILS_AfterColon;
+  FormatStyle::BILS_AfterColon;
   verifyFormat("class MyClass : public X {};",
StyleWithInheritanceBreakAfterColon);
   verifyFormat("class MyClass : public X, public Y {};",
@@ -2086,8 +2086,8 @@
   Style.NamespaceMacros.push_back("TESTSUITE");
 
   verifyFormat("namespace A { namespace B {\n"
-			   "}} // namespace A::B",
-			   Style);
+   "}} // namespace A::B",
+   Style);
 
   EXPECT_EQ("namespace out { namespace in {\n"
 "}} // namespace out::in",
@@ -4450,28 +4450,28 @@
   "SomeClass::Constructor() :\n"
   "aa(aa),\n"
   "aaa() {}",
-	  Style);
+  Style);
   verifyFormat("Constructor(aa ,\n"
"aa ) :\n"
"aa(aa) {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor() :\n"
"(aaa),\n"
"(aaa,\n"
" aaa),\n"
"aaa() {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor() :\n"
"(\n"
"a) {}",
-			   Style);
+   Style);
 
   verifyFormat("Constructor(int Parameter = 0) :\n"
"aa(a),\n"
"(a) {}",
-			   Style);
+   Style);
   verifyFormat("Constructor() :\n"
"aa(a), (b) {\n"
"}",
@@ -4479,7 +4479,7 @@
   verifyFormat("Constructor() :\n"
"a(\n"
"a(, )) {}",
-			   Style);
+   Style);
 
   // Here a line could be saved by splitting the second initializer onto two
   // lines, but that is not desirable.
@@ -4487,7 +4487,7 @@
"(),\n"
"aaa(aaa),\n"
"at() {}",
-			   Style);
+   Style);
 
   FormatStyle OnePerLine = Style;
   OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
@@ -4521,10 +4521,10 @@
   OnePerLine.BinPackParameters = false;
   verifyFormat(
   "Constructor() :\n"
-  "(\n"
-  "aaa().aaa(),\n"
-  "a) {}",
-  OnePerLine);
+   "(\n"
+   "aaa().aaa(),\n"
+   "a) {}",
+   OnePerLi

[PATCH] D67740: [Consumed] Refactor and improve diagnostics

2019-09-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Looks like this might benefit from being split into independent changes - the 
work related to templates (I haven't looked closely, but I assume that's fairly 
indivisible) and the work related to other diagnostics seems fairly separable - 
and maybe there's other pieces too.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3234
+  "consumed analysis attribute is attached to a "
+  "%select{static method|constructor}0 of class '%1'">,
+  InGroup, DefaultIgnore;

"member function" would be more correct than "method" here (the diagnostics in 
this file using the word "method" are mostly for Objective C things)

Are there other diagnostics that are similar? (like maybe function "const" - 
which can't be on non-member, static member, or ctors - wouldn't that be the 
same here? Where's the failure path for a non-member (free) function? Could it 
be unified with the static member/ctor case you're adding?)

Ah, looks like in Attr.td these attributes could be flagged 
"NonStaticCXXMethod" rather than "CXXMethod" - perhaps that narrower 
classification wasn't available when this was originally implemented. (then, I 
think, the attribute parsing logic will do the job of warning about the misuse 
and ignoring the attribute entirely)



Comment at: lib/Sema/SemaDeclAttr.cpp:1237-1240
+else if (auto *CX = dyn_cast(X))
+  return CX->getThisType()->getPointeeType();
+else
+  return cast(X)->getCallResultType();

Drop the else-after-return 
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67740/new/

https://reviews.llvm.org/D67740



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


[PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

2019-09-19 Thread Vedant Kumar via Phabricator via cfe-commits
vsk created this revision.
vsk added reviewers: teemperor, jasonmolenda, rjmccall.

LLDB may synthesize decls using asm labels. These decls cannot have a
mangle different than the one specified in the label name. I.e., the
mangle-suppression prefix should not be added.

Fixes an expression evaluation failure in lldb's TestVirtual.py on iOS.

rdar://45827323


https://reviews.llvm.org/D67774

Files:
  clang/include/clang/AST/ExternalASTSource.h
  clang/lib/AST/Mangle.cpp
  clang/test/Import/asm-label-mangle/Inputs/asm-label.cpp
  clang/test/Import/asm-label-mangle/test.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h

Index: lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
===
--- lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
+++ lldb/include/lldb/Symbol/ClangExternalASTSourceCommon.h
@@ -132,6 +132,11 @@
 
   static ClangExternalASTSourceCommon *Lookup(clang::ExternalASTSource *source);
 
+  // Request that no "hidden" prefix is added to the mangle for asm labels.
+  // LLDB synthesizes many decls as simple asm labels: these synthesized decls
+  // cannot have a mangle different than the one specified in the label name.
+  bool UseGlobalPrefixInAsmLabelMangle() override { return false; }
+
 private:
   typedef llvm::DenseMap MetadataMap;
 
Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -231,6 +231,18 @@
 
 namespace {
 
+// A mock ExternalASTMerger, used for testing purposes.
+class MockExternalASTMerger : public clang::ExternalASTMerger {
+public:
+  MockExternalASTMerger(const ImporterTarget &Target,
+llvm::ArrayRef Sources)
+  : clang::ExternalASTMerger(Target, Sources) {}
+
+  // Disallow use of a global prefix in the mangle for asm labels. This is
+  // required by lldb.
+  bool UseGlobalPrefixInAsmLabelMangle() override { return false; }
+};
+
 /// A container for a CompilerInstance (possibly with an ExternalASTMerger
 /// attached to its ASTContext).
 ///
@@ -265,7 +277,7 @@
   for (CIAndOrigins &Import : Imports)
 Sources.push_back({Import.getASTContext(), Import.getFileManager(),
Import.getOriginMap()});
-  auto ES = std::make_unique(Target, Sources);
+  auto ES = std::make_unique(Target, Sources);
   CI.getASTContext().setExternalSource(ES.release());
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
Index: clang/test/Import/asm-label-mangle/test.cpp
===
--- /dev/null
+++ clang/test/Import/asm-label-mangle/test.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-import-test -dump-ir -import %S/Inputs/asm-label.cpp -expression %s | FileCheck %s
+
+// The asm label mangle should not include a mangle-suppression prefix.
+
+// CHECK: define void @someotherfunc
+
+void expr() {
+  S1().func1();
+}
Index: clang/test/Import/asm-label-mangle/Inputs/asm-label.cpp
===
--- /dev/null
+++ clang/test/Import/asm-label-mangle/Inputs/asm-label.cpp
@@ -0,0 +1,4 @@
+struct S1 {
+  void func1() __asm("someotherfunc") {
+  }
+};
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -127,10 +127,13 @@
 // tricks normally used for producing aliases (PR9177). Fortunately the
 // llvm mangler on ELF is a nop, so we can just avoid adding the \01
 // marker.  We also avoid adding the marker if this is an alias for an
-// LLVM intrinsic.
+// LLVM intrinsic, or if the external AST source which provided the decl
+// opts out of this behavior.
 char GlobalPrefix =
 getASTContext().getTargetInfo().getDataLayout().getGlobalPrefix();
-if (GlobalPrefix && !ALA->getLabel().startswith("llvm."))
+ExternalASTSource *Source = getASTContext().getExternalSource();
+if (GlobalPrefix && !ALA->getLabel().startswith("llvm.") &&
+(!Source || Source->UseGlobalPrefixInAsmLabelMangle()))
   Out << '\01'; // LLVM IR Marker for __asm("foo")
 
 Out << ALA->getLabel();
Index: clang/include/clang/AST/ExternalASTSource.h
===
--- clang/include/clang/AST/ExternalASTSource.h
+++ clang/include/clang/AST/ExternalASTSource.h
@@ -104,6 +104,10 @@
   /// The default implementation of this method is a no-op.
   virtual Decl *GetExternalDecl(uint32_t ID);
 
+  /// Allow use of a global prefix (as defined in DataLayout) in the mangle
+  /// for asm labels. Defaults to true.
+  virtual bool UseGlobalPrefixInAsmLabelMangle() { return true; }
+
   /// Resolve a selector ID into a selector.
   ///
   //

[PATCH] D67632: [libTooling] Introduce new library of source-code builders.

2019-09-19 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 220901.
ymandel added a comment.

Added parens operator; cleaned comments and api a bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67632/new/

https://reviews.llvm.org/D67632

Files:
  clang/include/clang/Tooling/Refactoring/SourceCodeBuilders.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/SourceCodeBuilders.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/SourceCodeBuildersTest.cpp

Index: clang/unittests/Tooling/SourceCodeBuildersTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/SourceCodeBuildersTest.cpp
@@ -0,0 +1,200 @@
+//===- unittest/Tooling/SourceCodeBuildersTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/SourceCodeBuilders.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using MatchResult = MatchFinder::MatchResult;
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(StringRef StatementCode) {
+  return ("struct S { int field; }; "
+  "S operator+(const S &a, const S &b); "
+  "auto test_snippet = []{" +
+  StatementCode + "};")
+  .str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match one of the statements in
+// `StatementCode` exactly -- that is, produce exactly one match. However,
+// `StatementCode` may contain other statements not described by `Matcher`.
+static llvm::Optional matchStmt(StringRef StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+static void testPredicate(bool (*Pred)(const Expr &ExprNode), StringRef Snippet,
+  bool Expected) {
+  auto StmtMatch = matchStmt(Snippet, expr().bind("expr"));
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_EQ(Expected, Pred(*StmtMatch->Result.Nodes.getNodeAs("expr")));
+}
+
+TEST(SourceCodeBuildersTest, needParensAfterUnaryOperator) {
+  testPredicate(needParensAfterUnaryOperator, "3 + 5;", true);
+  testPredicate(needParensAfterUnaryOperator, "true ? 3 : 5;", true);
+
+  testPredicate(needParensAfterUnaryOperator, "int x; x;", false);
+  testPredicate(needParensAfterUnaryOperator, "int(3.0);", false);
+  testPredicate(needParensAfterUnaryOperator, "void f(); f();", false);
+  testPredicate(needParensAfterUnaryOperator, "int a[3]; a[0];", false);
+  testPredicate(needParensAfterUnaryOperator, "S x; x.field;", false);
+  testPredicate(needParensAfterUnaryOperator, "int x = 1; --x;", false);
+  testPredicate(needParensAfterUnaryOperator, "int x = 1; -x;", false);
+}
+
+TEST(SourceCodeBuildersTest, mayNeedParens) {
+  testPredicate(mayNeedParens, "3 + 5;", true);
+  testPredicate(mayNeedParens, "true ? 3 : 5;", true);
+  testPredicate(mayNeedParens, "int x = 1; --x;", true);
+  testPredicate(mayNeedParens, "int x = 1; -x;", true);
+
+  testPredicate(mayNeedParens, "int x; x;", false);
+  testPredicate(mayNeedParens, "int(3.0);", false);
+  testPredicate(mayNeedParens, "void f(); f();", false);
+  testPredicate(mayNeedParens, "int a[3]; a[0];", false);
+  testPredicate(mayNeedParens, "S x; x.field;", false);
+}
+
+static void testBuilder(std::string (*Builder)(const Expr &ExprNode,
+

[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

2019-09-19 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added a reviewer: aaron.ballman.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.

The warnings now in -Wformat-type-confusion don't align with how we interpret 
'pedantic' in clang, and don't belong in `-pedantic`.

Suggested by Aaron here: D66856 

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D67775

Files:
  clang/include/clang/AST/FormatString.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/format-bool.c
  clang/test/Sema/format-strings-pedantic.c
  clang/test/Sema/format-type-confusion.c

Index: clang/test/Sema/format-type-confusion.c
===
--- /dev/null
+++ clang/test/Sema/format-type-confusion.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9.0 -fsyntax-only -verify -Wno-format -Wformat-type-confusion %s
+
+__attribute__((format(__printf__, 1, 2)))
+int printf(const char *msg, ...);
+
+#define FMT "%hd %hu %d %u %hhd %hhu %c"
+
+int main() {
+  _Bool b = 0;
+  printf(FMT,
+ b, // expected-warning {{format specifies type 'short' but the argument has type '_Bool'}}
+ b, // expected-warning {{format specifies type 'unsigned short' but the argument has type '_Bool'}}
+ b, b, b, b, b);
+
+  unsigned char uc = 0;
+  printf(FMT,
+ uc, // expected-warning {{format specifies type 'short' but the argument has type 'unsigned char'}}
+ uc, // expected-warning {{format specifies type 'unsigned short' but the argument has type 'unsigned char'}}
+ uc, uc, uc, uc, uc);
+
+  signed char sc = 0;
+  printf(FMT,
+ sc, // expected-warning {{format specifies type 'short' but the argument has type 'signed char'}}
+ sc, // expected-warning {{format specifies type 'unsigned short' but the argument has type 'signed char'}}
+ sc, sc, sc, sc, sc);
+}
Index: clang/test/Sema/format-strings-pedantic.c
===
--- clang/test/Sema/format-strings-pedantic.c
+++ clang/test/Sema/format-strings-pedantic.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-pedantic -isystem %S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-type-confusion %s
 
 int printf(const char *restrict, ...);
 
Index: clang/test/Sema/format-bool.c
===
--- clang/test/Sema/format-bool.c
+++ clang/test/Sema/format-bool.c
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool
 // RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool
 // RUN: %clang_cc1 -xobjective-c %s -verify -DBOOL=_Bool
-// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-pedantic -DPEDANTIC
-// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-pedantic -DPEDANTIC
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-type-confusion -DTYPE_CONF
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-type-confusion -DTYPE_CONF
 
 __attribute__((format(__printf__, 1, 2)))
 int p(const char *fmt, ...);
@@ -22,13 +22,13 @@
 int main() {
   p("%d", b);
   p("%hd", b);
-#ifdef PEDANTIC
+#ifdef TYPE_CONF
   // expected-warning@-2 {{format specifies type 'short' but the argument has type}}
 #endif
   p("%hhd", b);
   p("%u", b);
   p("%hu", b);
-#ifdef PEDANTIC
+#ifdef TYPE_CONF
   // expected-warning@-2 {{format specifies type 'unsigned short' but the argument has type}}
 #endif
   p("%hhu", b);
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8135,9 +8135,7 @@
 return true;
   }
 
-  const analyze_printf::ArgType::MatchKind Match =
-  AT.matchesType(S.Context, ExprTy);
-  bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
+  analyze_printf::ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
   if (Match == analyze_printf::ArgType::Match)
 return true;
 
@@ -8161,8 +8159,10 @@
 AT.matchesType(S.Context, ExprTy);
 if (ImplicitMatch == analyze_printf::ArgType::Match)
   return true;
-if (ImplicitMatch == analyze_printf::ArgType::NoMatchPedantic)
-  Pedantic = true;
+if (ImplicitMatch == ArgType::NoMatchPedantic)
+  Match = ArgType::NoMatchPedantic;
+if (ImplicitMatch == ArgType::NoMatchTypeConfusion)
+  Match = ArgType::NoMatchTypeConfusion;
   }
 }
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
@@ -8224,7 +8224,7 @@
   if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
   (AT.isSizeT() || AT.isPtrdiffT()) &&
   AT.matchesType(S.Context, CastTy))
-Pedantic = true;
+Matc

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2019-09-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Is it just me or phabricator somehow refuses to display the changes since the 
last diff here? That's probably the commit diff is involved somehow. So i'm 
confused what exactly has changed >.<


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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


[PATCH] D67635: Fix for stringized function-macro args continued across lines

2019-09-19 Thread Kousik Kumar via Phabricator via cfe-commits
kousikk added a comment.

Thanks! I will need you to merge this one too!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67635/new/

https://reviews.llvm.org/D67635



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


[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2019-09-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done.
Szelethus added a comment.

In D54823#1675604 , @NoQ wrote:

> Is it just me or phabricator somehow refuses to display the changes since the 
> last diff here? That's probably the commit diff is involved somehow. So i'm 
> confused what exactly has changed >.<


File name change, I'm using the monorepo now.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3137
+  if (RS->isAllocated() || RS->isAllocatedOfSizeZero())
+if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS))
+  State = State->set(sym, RefState::getEscaped(RS));

This was the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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


[PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

2019-09-19 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/AST/Mangle.cpp:137
+(!Source || Source->UseGlobalPrefixInAsmLabelMangle()))
   Out << '\01'; // LLVM IR Marker for __asm("foo")
 

This is one of those bugs where looking at the code really reveals a lot of 
problematic behavior.

`AsmLabelAttr` is generally assumed to carry a literal label, i.e. it should 
not have the global prefix added to it.  We should not be changing that 
assumption globally just based on whether an `ExternalASTSource` has been set 
up; that's going to break in any sort of non-uniform situation, e.g. if an LLDB 
user writes a declaration with an asm label, or if we import a Clang module 
where a declaration has an asm label.  Either `ExternalASTSource`s should be 
putting the real (prefixed) symbol name in the `AsmLabelAttr`, or they should 
be making `AsmLabelAttr`s that are different somehow, e.g. by giving 
`AsmLabelAttr` a flag (which doesn't need to be spellable in the source 
language) saying whether the label is literal or subject to global prefixing.

Separately, it seems to me that if the `AsmLabelAttr` is literal but the label 
happens to include the global prefix, we should strip the prefix from the label 
and not add a `\01` so that we get a consistent symbol name in LLVM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67774/new/

https://reviews.llvm.org/D67774



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


[PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

2019-09-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

I wonder what's the motivation for making this a setting in the 
ExternalASTSource? Opposed to e.g. storing a bit in AsmLabelAttr, which we 
could squeeze in by maybe stealing a bit from `labelLength`? Having this as a 
global setting in the ExternalASTSource seems like it could end up in a lot of 
confusion when we refactor our ExternalASTSources and this unexpectedly breaks. 
Especially since multiplexing ExternalASTSources is a thing people do 
(including LLDB itself), so if one source wants this feature on and the other 
doesn't, then we are in a dead end.

Also I wonder what happens with our real declarations we get from Clang modules 
(ObjC for now and C++ in the future). They now also share this setting even 
though they could use asm labels for legitimate reasons (even though I'm not 
sure I ever saw one being used in practice).

One minor bug I found: You need to turn this on in  `SemaSourceWithPriorities` 
and maybe also `ExternalASTSourceWrapper` (see ASTUtils.h) otherwise this seems 
to regress when using C++ modules and mangling stuff for an expression (I 
couldn't use `ClangExternalASTSourceCommon` there as we needed a 
clang::SemaSource).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67774/new/

https://reviews.llvm.org/D67774



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


[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

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

In retrospect, I would have made this patch a little more fragmented (its 
almost a year old, does it beat the revival of the symbolreaper patch?), but it 
would be a painful chore to separate at this point, if you dont mind. I have a 
couple more cooking in the cauldron that are more digestible, so, lesson 
learned :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 220907.
yaxunl added a comment.

simplify logic by Artem's comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509

Files:
  lib/Sema/SemaCUDA.cpp
  test/SemaCUDA/default-ctor.cu

Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:-fcuda-is-device -verify -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only \
+// RUN:-verify -verify-ignore-unexpected=note %s
+
+#include "Inputs/cuda.h"
+
+struct In { In() = default; };
+struct InD { __device__ InD() = default; };
+struct InH { __host__ InH() = default; };
+struct InHD { __host__ __device__ InHD() = default; };
+
+struct Out { Out(); };
+struct OutD { __device__ OutD(); };
+struct OutH { __host__ OutH(); };
+struct OutHD { __host__ __device__ OutHD(); };
+
+Out::Out() = default;
+__device__ OutD::OutD() = default;
+__host__ OutH::OutH() = default;
+__host__ __device__ OutHD::OutHD() = default;
+
+__device__ void fd() {
+  In in;
+  InD ind;
+  InH inh; // expected-error{{no matching constructor for initialization of 'InH'}}
+  InHD inhd;
+  Out out; // expected-error{{no matching constructor for initialization of 'Out'}}
+  OutD outd;
+  OutH outh; // expected-error{{no matching constructor for initialization of 'OutH'}}
+  OutHD outhd;
+}
+
+__host__ void fh() {
+  In in;
+  InD ind; // expected-error{{no matching constructor for initialization of 'InD'}}
+  InH inh;
+  InHD inhd;
+  Out out;
+  OutD outd; // expected-error{{no matching constructor for initialization of 'OutD'}}
+  OutH outh;
+  OutHD outhd;
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -267,6 +267,17 @@
CXXMethodDecl *MemberDecl,
bool ConstRHS,
bool Diagnose) {
+  // If the defaulted special member is defined lexically outside of its
+  // owning class, or the special member already has explicit device or host
+  // attributes, do not infer.
+  bool InClass = MemberDecl->getLexicalParent() == MemberDecl->getParent();
+  bool HasExpAttr = (MemberDecl->hasAttr() &&
+ !MemberDecl->getAttr()->isImplicit()) ||
+(MemberDecl->hasAttr() &&
+ !MemberDecl->getAttr()->isImplicit());
+  if (!InClass || HasExpAttr)
+return false;
+
   llvm::Optional InferredTarget;
 
   // We're going to invoke special member lookup; mark that these special
@@ -371,21 +382,26 @@
 }
   }
 
+  bool NeedsH = true, NeedsD = true;
+  bool HasH = MemberDecl->hasAttr();
+  bool HasD = MemberDecl->hasAttr();
+
+  // If no target was inferred, mark this member as __host__ __device__;
+  // it's the least restrictive option that can be invoked from any target.
   if (InferredTarget.hasValue()) {
-if (InferredTarget.getValue() == CFT_Device) {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-} else if (InferredTarget.getValue() == CFT_Host) {
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-} else {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-}
-  } else {
-// If no target was inferred, mark this member as __host__ __device__;
-// it's the least restrictive option that can be invoked from any target.
+if (InferredTarget.getValue() == CFT_Device)
+  NeedsH = false;
+else if (InferredTarget.getValue() == CFT_Host)
+  NeedsD = false;
+  }
+
+  // We either setting attributes first time, or the inferred ones must match
+  // previously set ones.
+  assert(!(HasD || HasH) || (NeedsD == HasD && NeedsH == HasH));
+  if (NeedsD && !HasD)
 MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
+  if (NeedsH && !HasH)
 MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-  }
 
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Sema/SemaCUDA.cpp:386-387
+  bool NeedsH = true, NeedsD = true;
+  bool HasH = MemberDecl->hasAttr();
+  bool HasD = MemberDecl->hasAttr();
+

Nice.
Now these can be moved above `HasExpAttr` and then used in its initializer to 
make it shorter.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


r372353 - Revert "[CUDA][HIP] Fix typo in `BestViableFunction`"

2019-09-19 Thread Mitch Phillips via cfe-commits
Author: hctim
Date: Thu Sep 19 14:11:28 2019
New Revision: 372353

URL: http://llvm.org/viewvc/llvm-project?rev=372353&view=rev
Log:
Revert "[CUDA][HIP] Fix typo in `BestViableFunction`"

Broke the msan buildbots (see comments on rL372318 for more details).

This reverts commit eb231d15825ac345b546f4c99372d1cac8f14f02.

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/SemaCUDA/function-overload.cu
cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=372353&r1=372352&r2=372353&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Sep 19 14:11:28 2019
@@ -9422,19 +9422,17 @@ OverloadCandidateSet::BestViableFunction
 const FunctionDecl *Caller = dyn_cast(S.CurContext);
 bool ContainsSameSideCandidate =
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
-  // Consider viable function only.
-  return Cand->Viable && Cand->Function &&
+  return Cand->Function &&
  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
  Sema::CFP_SameSide;
 });
 if (ContainsSameSideCandidate) {
-  // Clear viable flag for WrongSide varible candidates.
-  llvm::for_each(Candidates, [&](OverloadCandidate *Cand) {
-if (Cand->Viable && Cand->Function &&
-S.IdentifyCUDAPreference(Caller, Cand->Function) ==
-Sema::CFP_WrongSide)
-  Cand->Viable = false;
-  });
+  auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
+return Cand->Function &&
+   S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+   Sema::CFP_WrongSide;
+  };
+  llvm::erase_if(Candidates, IsWrongSideCandidate);
 }
   }
 

Modified: cfe/trunk/test/SemaCUDA/function-overload.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/function-overload.cu?rev=372353&r1=372352&r2=372353&view=diff
==
--- cfe/trunk/test/SemaCUDA/function-overload.cu (original)
+++ cfe/trunk/test/SemaCUDA/function-overload.cu Thu Sep 19 14:11:28 2019
@@ -402,20 +402,3 @@ __host__ void test_host_template_overloa
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ 
variant.
 }
-
-// Two classes with `operator-` defined. One of them is device only.
-struct C1;
-struct C2;
-__device__
-int operator-(const C1 &x, const C1 &y);
-int operator-(const C2 &x, const C2 &y);
-
-template 
-__host__ __device__ int constexpr_overload(const T &x, const T &y) {
-  return x - y;
-}
-
-// Verify that function overloading doesn't prune candidate wrongly.
-int test_constexpr_overload(C2 &x, C2 &y) {
-  return constexpr_overload(x, y);
-}

Modified: cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu?rev=372353&r1=372352&r2=372353&view=diff
==
--- cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu (original)
+++ cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu Thu Sep 
19 14:11:28 2019
@@ -74,13 +74,11 @@ struct B4_with_device_copy_ctor {
 struct C4_with_collision : A4_with_host_copy_ctor, B4_with_device_copy_ctor {
 };
 
-// expected-note@-3 {{candidate constructor (the implicit copy constructor) 
not viable: call to invalid function from __host__ function}}
-// expected-note@-4 {{implicit copy constructor inferred target collision: 
call to both __host__ and __device__ members}}
-// expected-note@-5 {{candidate constructor (the implicit default constructor) 
not viable: requires 0 arguments, but 1 was provided}}
+// expected-note@-3 {{copy constructor of 'C4_with_collision' is implicitly 
deleted because base class 'B4_with_device_copy_ctor' has no copy constructor}}
 
 void hostfoo4() {
   C4_with_collision c;
-  C4_with_collision c2 = c; // expected-error {{no matching constructor for 
initialization of 'C4_with_collision'}}
+  C4_with_collision c2 = c; // expected-error {{call to implicitly-deleted 
copy constructor of 'C4_with_collision'}}
 }
 
 
//--


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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 220912.
yaxunl added a comment.

revise by Artem's comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509

Files:
  lib/Sema/SemaCUDA.cpp
  test/SemaCUDA/default-ctor.cu

Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only \
+// RUN:-fcuda-is-device -verify -verify-ignore-unexpected=note %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -fsyntax-only \
+// RUN:-verify -verify-ignore-unexpected=note %s
+
+#include "Inputs/cuda.h"
+
+struct In { In() = default; };
+struct InD { __device__ InD() = default; };
+struct InH { __host__ InH() = default; };
+struct InHD { __host__ __device__ InHD() = default; };
+
+struct Out { Out(); };
+struct OutD { __device__ OutD(); };
+struct OutH { __host__ OutH(); };
+struct OutHD { __host__ __device__ OutHD(); };
+
+Out::Out() = default;
+__device__ OutD::OutD() = default;
+__host__ OutH::OutH() = default;
+__host__ __device__ OutHD::OutHD() = default;
+
+__device__ void fd() {
+  In in;
+  InD ind;
+  InH inh; // expected-error{{no matching constructor for initialization of 'InH'}}
+  InHD inhd;
+  Out out; // expected-error{{no matching constructor for initialization of 'Out'}}
+  OutD outd;
+  OutH outh; // expected-error{{no matching constructor for initialization of 'OutH'}}
+  OutHD outhd;
+}
+
+__host__ void fh() {
+  In in;
+  InD ind; // expected-error{{no matching constructor for initialization of 'InD'}}
+  InH inh;
+  InHD inhd;
+  Out out;
+  OutD outd; // expected-error{{no matching constructor for initialization of 'OutD'}}
+  OutH outh;
+  OutHD outhd;
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -267,6 +267,18 @@
CXXMethodDecl *MemberDecl,
bool ConstRHS,
bool Diagnose) {
+  // If the defaulted special member is defined lexically outside of its
+  // owning class, or the special member already has explicit device or host
+  // attributes, do not infer.
+  bool InClass = MemberDecl->getLexicalParent() == MemberDecl->getParent();
+  bool HasH = MemberDecl->hasAttr();
+  bool HasD = MemberDecl->hasAttr();
+  bool HasExplicitAttr =
+  (HasD && !MemberDecl->getAttr()->isImplicit()) ||
+  (HasH && !MemberDecl->getAttr()->isImplicit());
+  if (!InClass || HasExplicitAttr)
+return false;
+
   llvm::Optional InferredTarget;
 
   // We're going to invoke special member lookup; mark that these special
@@ -371,21 +383,24 @@
 }
   }
 
+
+  // If no target was inferred, mark this member as __host__ __device__;
+  // it's the least restrictive option that can be invoked from any target.
+  bool NeedsH = true, NeedsD = true;
   if (InferredTarget.hasValue()) {
-if (InferredTarget.getValue() == CFT_Device) {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-} else if (InferredTarget.getValue() == CFT_Host) {
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-} else {
-  MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
-  MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-}
-  } else {
-// If no target was inferred, mark this member as __host__ __device__;
-// it's the least restrictive option that can be invoked from any target.
+if (InferredTarget.getValue() == CFT_Device)
+  NeedsH = false;
+else if (InferredTarget.getValue() == CFT_Host)
+  NeedsD = false;
+  }
+
+  // We either setting attributes first time, or the inferred ones must match
+  // previously set ones.
+  assert(!(HasD || HasH) || (NeedsD == HasD && NeedsH == HasH));
+  if (NeedsD && !HasD)
 MemberDecl->addAttr(CUDADeviceAttr::CreateImplicit(Context));
+  if (NeedsH && !HasH)
 MemberDecl->addAttr(CUDAHostAttr::CreateImplicit(Context));
-  }
 
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r372356 - [CUDA][HIP] Re-apply part of r372318.

2019-09-19 Thread Michael Liao via cfe-commits
Author: hliao
Date: Thu Sep 19 14:26:18 2019
New Revision: 372356

URL: http://llvm.org/viewvc/llvm-project?rev=372356&view=rev
Log:
[CUDA][HIP] Re-apply part of r372318.

- r372318 causes violation of `use-of-uninitialized-value` detected by
  MemorySanitizer. Once `Viable` field is set to false, `FailureKind`
  needs setting as well as it will be checked during destruction if
  `Viable` is not true.
- Revert the part trying to skip `std::vector` erasing.

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/SemaCUDA/function-overload.cu
cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=372356&r1=372355&r2=372356&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Sep 19 14:26:18 2019
@@ -9422,13 +9422,15 @@ OverloadCandidateSet::BestViableFunction
 const FunctionDecl *Caller = dyn_cast(S.CurContext);
 bool ContainsSameSideCandidate =
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
-  return Cand->Function &&
+  // Check viable function only.
+  return Cand->Viable && Cand->Function &&
  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
  Sema::CFP_SameSide;
 });
 if (ContainsSameSideCandidate) {
   auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
-return Cand->Function &&
+// Check viable function only to avoid unnecessary data copying/moving.
+return Cand->Viable && Cand->Function &&
S.IdentifyCUDAPreference(Caller, Cand->Function) ==
Sema::CFP_WrongSide;
   };

Modified: cfe/trunk/test/SemaCUDA/function-overload.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/function-overload.cu?rev=372356&r1=372355&r2=372356&view=diff
==
--- cfe/trunk/test/SemaCUDA/function-overload.cu (original)
+++ cfe/trunk/test/SemaCUDA/function-overload.cu Thu Sep 19 14:26:18 2019
@@ -402,3 +402,20 @@ __host__ void test_host_template_overloa
 __device__ void test_device_template_overload() {
   template_overload(1); // OK. Attribute-based overloading picks __device__ 
variant.
 }
+
+// Two classes with `operator-` defined. One of them is device only.
+struct C1;
+struct C2;
+__device__
+int operator-(const C1 &x, const C1 &y);
+int operator-(const C2 &x, const C2 &y);
+
+template 
+__host__ __device__ int constexpr_overload(const T &x, const T &y) {
+  return x - y;
+}
+
+// Verify that function overloading doesn't prune candidate wrongly.
+int test_constexpr_overload(C2 &x, C2 &y) {
+  return constexpr_overload(x, y);
+}

Modified: cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu?rev=372356&r1=372355&r2=372356&view=diff
==
--- cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu (original)
+++ cfe/trunk/test/SemaCUDA/implicit-member-target-collision-cxx11.cu Thu Sep 
19 14:26:18 2019
@@ -74,11 +74,13 @@ struct B4_with_device_copy_ctor {
 struct C4_with_collision : A4_with_host_copy_ctor, B4_with_device_copy_ctor {
 };
 
-// expected-note@-3 {{copy constructor of 'C4_with_collision' is implicitly 
deleted because base class 'B4_with_device_copy_ctor' has no copy constructor}}
+// expected-note@-3 {{candidate constructor (the implicit copy constructor) 
not viable: call to invalid function from __host__ function}}
+// expected-note@-4 {{implicit copy constructor inferred target collision: 
call to both __host__ and __device__ members}}
+// expected-note@-5 {{candidate constructor (the implicit default constructor) 
not viable: requires 0 arguments, but 1 was provided}}
 
 void hostfoo4() {
   C4_with_collision c;
-  C4_with_collision c2 = c; // expected-error {{call to implicitly-deleted 
copy constructor of 'C4_with_collision'}}
+  C4_with_collision c2 = c; // expected-error {{no matching constructor for 
initialization of 'C4_with_collision'}}
 }
 
 
//--


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


[PATCH] D67635: Fix for stringized function-macro args continued across lines

2019-09-19 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

In D67635#1675606 , @kousikk wrote:

> Thanks! I will need you to merge this one too!


Sure, I will commit it today.

I think you should obtain commit access for your future patches/commits. You 
can follow the instructions here: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67635/new/

https://reviews.llvm.org/D67635



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


[PATCH] D67509: [CUDA][HIP] Fix hostness of defaulted constructor

2019-09-19 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.

LGTM. Thank you!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67509/new/

https://reviews.llvm.org/D67509



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


[PATCH] D67052: Add reference type transformation builtins

2019-09-19 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added inline comments.



Comment at: clang/include/clang/AST/Type.h:4353
+RemoveReferenceType,
+AddRValueType,
+AddLValueType

This should say ref in the name.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1094
+return DeclSpec::TST_removeReferenceType;
+  default:
+assert(false && "Not a reference type specifier");

Don't use default switch cases. It suppresses non-covered switch warnings, 
which we want fire when a new enumerator is added.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



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


[PATCH] D67052: Add reference type transformation builtins

2019-09-19 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:2606
+  DeclSpec::TST ReferenceTransformTokToDeclSpec();
+  void ParseAddReferenceTypeSpecifier(DeclSpec &DS);
   void ParseAtomicSpecifier(DeclSpec &DS);

These methods are generalized more in D67588.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



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


r372359 - Model converted constant expressions as full-expressions.

2019-09-19 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Sep 19 15:00:16 2019
New Revision: 372359

URL: http://llvm.org/viewvc/llvm-project?rev=372359&view=rev
Log:
Model converted constant expressions as full-expressions.

This is groundwork for C++20's P0784R7, where non-trivial destructors
can be constexpr, so we need ExprWithCleanups markers in constant
expressions.

No functionality change intended.

Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=372359&r1=372358&r2=372359&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu Sep 19 15:00:16 2019
@@ -4678,8 +4678,10 @@ void Parser::ParseEnumBody(SourceLocatio
 ExprResult AssignedVal;
 EnumAvailabilityDiags.emplace_back(*this);
 
+EnterExpressionEvaluationContext ConstantEvaluated(
+Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
 if (TryConsumeToken(tok::equal, EqualLoc)) {
-  AssignedVal = ParseConstantExpression();
+  AssignedVal = ParseConstantExpressionInExprEvalContext();
   if (AssignedVal.isInvalid())
 SkipUntil(tok::comma, tok::r_brace, StopBeforeMatch);
 }

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=372359&r1=372358&r2=372359&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Sep 19 15:00:16 2019
@@ -257,8 +257,18 @@ isPointerConversionToVoidPointer(ASTCont
 
 /// Skip any implicit casts which could be either part of a narrowing 
conversion
 /// or after one in an implicit conversion.
-static const Expr *IgnoreNarrowingConversion(const Expr *Converted) {
-  while (const ImplicitCastExpr *ICE = dyn_cast(Converted)) {
+static const Expr *IgnoreNarrowingConversion(ASTContext &Ctx,
+ const Expr *Converted) {
+  // We can have cleanups wrapping the converted expression; these need to be
+  // preserved so that destructors run if necessary.
+  if (auto *EWC = dyn_cast(Converted)) {
+Expr *Inner =
+const_cast(IgnoreNarrowingConversion(Ctx, EWC->getSubExpr()));
+return ExprWithCleanups::Create(Ctx, Inner, EWC->cleanupsHaveSideEffects(),
+EWC->getObjects());
+  }
+
+  while (auto *ICE = dyn_cast(Converted)) {
 switch (ICE->getCastKind()) {
 case CK_NoOp:
 case CK_IntegralCast:
@@ -332,7 +342,7 @@ NarrowingKind StandardConversionSequence
   if (IgnoreFloatToIntegralConversion)
 return NK_Not_Narrowing;
   llvm::APSInt IntConstantValue;
-  const Expr *Initializer = IgnoreNarrowingConversion(Converted);
+  const Expr *Initializer = IgnoreNarrowingConversion(Ctx, Converted);
   assert(Initializer && "Unknown conversion expression");
 
   // If it's value-dependent, we can't tell whether it's narrowing.
@@ -370,7 +380,7 @@ NarrowingKind StandardConversionSequence
 if (FromType->isRealFloatingType() && ToType->isRealFloatingType() &&
 Ctx.getFloatingTypeOrder(FromType, ToType) == 1) {
   // FromType is larger than ToType.
-  const Expr *Initializer = IgnoreNarrowingConversion(Converted);
+  const Expr *Initializer = IgnoreNarrowingConversion(Ctx, Converted);
 
   // If it's value-dependent, we can't tell whether it's narrowing.
   if (Initializer->isValueDependent())
@@ -416,7 +426,7 @@ NarrowingKind StandardConversionSequence
 (FromSigned && !ToSigned)) {
   // Not all values of FromType can be represented in ToType.
   llvm::APSInt InitializerValue;
-  const Expr *Initializer = IgnoreNarrowingConversion(Converted);
+  const Expr *Initializer = IgnoreNarrowingConversion(Ctx, Converted);
 
   // If it's value-dependent, we can't tell whether it's narrowing.
   if (Initializer->isValueDependent())
@@ -5465,6 +5475,14 @@ static ExprResult CheckConvertedConstant
   if (Result.isInvalid())
 return Result;
 
+  // C++2a [intro.execution]p5:
+  //   A full-expression is [...] a constant-expression [...]
+  Result =
+  S.ActOnFinishFullExpr(Result.get(), From->getExprLoc(),
+/*DiscardedValue=*/false, /*IsConstexpr=*/true);
+  if (Result.isInvalid())
+return Result;
+
   // Check for a narrowing implicit conversion.
   APValue PreNarrowingValue;
   QualType PreNarrowingType;

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=372359&r1=372358&r2=372359&view=diff
==
--- cfe/tru

[PATCH] D67052: Add reference type transformation builtins

2019-09-19 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1099
+
+void Parser::ParseAddReferenceTypeSpecifier(DeclSpec &DS) {
+  DeclSpec::TST ReferenceTransformTST = ReferenceTransformTokToDeclSpec();

I think this should be generalized and merged with 
`ParseUnderlyingTypeSpecifier`.

Maybe called `ParseTransformationTypeSpecifier`.



Comment at: clang/lib/Sema/SemaType.cpp:1612
 break;
+  case DeclSpec::TST_addLValueReferenceType:
+  case DeclSpec::TST_addRValueReferenceType:

This should be merged with the above case. 



Comment at: clang/lib/Sema/SemaType.cpp:5490
+  // TODO: Should we check something like 
"IsUnaryTypeTransfrom(DS.getTypeSpecTypeLoc())"?
+  // assert(DS.getTypeSpecType() == DeclSpec::TST_underlyingType);
   TL.setKWLoc(DS.getTypeSpecTypeLoc());

`DS.getTypeSpecType() >= DeclSpec::TST_underlyingType && DS.getTypeSpecType() 
<= TST_removeReferenceType` would correct this assertion. 



Comment at: clang/test/SemaCXX/add_reference.cpp:66
+static_assert(test>::value, "");
+static_assert(test>::value, "");

You should test a lot more types here.  reference types, const types, pointer 
types, function types, non-referenceable types.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



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


[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

This isn't covered by any timer; if you look in `BackendUtil.cpp`,
`EmitAssemblyHelper` actually has `CodeGenerationTime("codegen", "Code 
Generation Time")` timer.



Comment at: llvm/trunk/lib/IR/LegacyPassManager.cpp:1686
 
+  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function &F : M)

I think this may be the wrong place for this.
This includes the entirety of the pipeline, including all of llvm back-end 
stuff.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58675/new/

https://reviews.llvm.org/D58675



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


[PATCH] D67635: Fix for stringized function-macro args continued across lines

2019-09-19 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372360: Fix for stringized function-macro args continued 
across lines (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67635?vs=220390&id=220914#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67635/new/

https://reviews.llvm.org/D67635

Files:
  cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp


Index: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -246,9 +246,12 @@
 
 static const char *reverseOverSpaces(const char *First, const char *Last) {
   assert(First <= Last);
-  while (First != Last && isHorizontalWhitespace(Last[-1]))
+  const char *PrevLast = Last;
+  while (First != Last && isHorizontalWhitespace(Last[-1])) {
+PrevLast = Last;
 --Last;
-  return Last;
+  }
+  return PrevLast;
 }
 
 static void skipLineComment(const char *&First, const char *const End) {
Index: cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -157,19 +157,19 @@
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\t)\tcon \t tent\t", Out));
-  EXPECT_STREQ("#define MACRO() con \t tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \t tent\t\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\f)\fcon \f tent\f", Out));
-  EXPECT_STREQ("#define MACRO() con \f tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \f tent\f\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\v)\vcon \v tent\v", Out));
-  EXPECT_STREQ("#define MACRO() con \v tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \v tent\v\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO \t\v\f\v\t con\f\t\vtent\v\f \v", Out));
-  EXPECT_STREQ("#define MACRO con\f\t\vtent\n", Out.data());
+  EXPECT_STREQ("#define MACRO con\f\t\vtent\v\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgs) {
@@ -187,7 +187,7 @@
"call((a),  \\\n"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -200,7 +200,17 @@
"call((a),  \\\r"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgsStringize) {
+  SmallVector Out;
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(a,b) \\\n"
+"#a \\\n"
+"#b",
+Out));
+  EXPECT_STREQ("#define MACRO(a,b) #a #b\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -213,7 +223,7 @@
"call((a),  \\\r\n"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -226,7 +236,7 @@
"call((a),  \\\n\r"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineNumber) {


Index: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -246,9 +246,12 @@
 
 stat

r372360 - Fix for stringized function-macro args continued across lines

2019-09-19 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Thu Sep 19 15:39:24 2019
New Revision: 372360

URL: http://llvm.org/viewvc/llvm-project?rev=372360&view=rev
Log:
Fix for stringized function-macro args continued across lines

In case of certain #define'd macros, there's a space just before line 
continuation
that the minimized-source lexer was missing to include, resulting in invalid 
stringize.

Patch by: kousikk (Kousik Kumar)

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

Modified:
cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Modified: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp?rev=372360&r1=372359&r2=372360&view=diff
==
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp (original)
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp Thu Sep 19 
15:39:24 2019
@@ -246,9 +246,12 @@ static void skipToNewlineRaw(const char
 
 static const char *reverseOverSpaces(const char *First, const char *Last) {
   assert(First <= Last);
-  while (First != Last && isHorizontalWhitespace(Last[-1]))
+  const char *PrevLast = Last;
+  while (First != Last && isHorizontalWhitespace(Last[-1])) {
+PrevLast = Last;
 --Last;
-  return Last;
+  }
+  return PrevLast;
 }
 
 static void skipLineComment(const char *&First, const char *const End) {

Modified: cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp?rev=372360&r1=372359&r2=372360&view=diff
==
--- cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp 
(original)
+++ cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp Thu Sep 
19 15:39:24 2019
@@ -157,19 +157,19 @@ TEST(MinimizeSourceToDependencyDirective
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\t)\tcon \t tent\t", Out));
-  EXPECT_STREQ("#define MACRO() con \t tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \t tent\t\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\f)\fcon \f tent\f", Out));
-  EXPECT_STREQ("#define MACRO() con \f tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \f tent\f\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO(\v)\vcon \v tent\v", Out));
-  EXPECT_STREQ("#define MACRO() con \v tent\n", Out.data());
+  EXPECT_STREQ("#define MACRO() con \v tent\v\n", Out.data());
 
   ASSERT_FALSE(minimizeSourceToDependencyDirectives(
   "#define MACRO \t\v\f\v\t con\f\t\vtent\v\f \v", Out));
-  EXPECT_STREQ("#define MACRO con\f\t\vtent\n", Out.data());
+  EXPECT_STREQ("#define MACRO con\f\t\vtent\v\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgs) {
@@ -187,7 +187,7 @@ TEST(MinimizeSourceToDependencyDirective
"call((a),  \\\n"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -200,7 +200,17 @@ TEST(MinimizeSourceToDependencyDirective
"call((a),  \\\r"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, DefineMultilineArgsStringize) {
+  SmallVector Out;
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives("#define MACRO(a,b) \\\n"
+"#a \\\n"
+"#b",
+Out));
+  EXPECT_STREQ("#define MACRO(a,b) #a #b\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -213,7 +223,7 @@ TEST(MinimizeSourceToDependencyDirective
"call((a),  \\\r\n"
" (b))",
Out));
-  EXPECT_STREQ("#define MACRO(a,b) call((a),(b))\n", Out.data());
+  EXPECT_STREQ("#define MACRO(a,b) call((a), (b))\n", Out.data());
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest,
@@ -226,7 +236,7 @@ TEST(MinimizeSourceToDependencyDirective
"c

[PATCH] D67743: [Consumed] Treat by-value class arguments as consuming by default, like rvalue refs.

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
comex added a comment.

In D67743#1675533 , @dblaikie wrote:

> "Also, fix the order of if statements so that an explicit return_typestate 
> annotation takes precedence over the default behavior for rvalue refs."
>
> I'd probably have split that out into a separate patch - in part to discuss 
> the design implications of that. I have some doubts about supporting 
> non-consumed after passing by rvalue ref, but don't feel too strongly & the 
> alternative would be having a warning about the attribute being ignored, etc 
> - when it has a fairly clear meaning if it is there, just not sure people 
> 'should' be doing that.


Fair enough.  I agree there's not much reason to do that, but it also seems 
pretty harmless to respect the user's choice.  Silently ignoring the attribute 
as before is obviously wrong, so the alternative would be adding a diagnostic 
for that case, but it doesn't seem worth it...

> unless these are testing something noteworthy about them being static 
> functions I'd probably suggest making them non-members like the other test 
> functions below, for consistency (otherwise the inconsistency tends to raise 
> the question of "what is significant about this difference?")

Okay, I'll change them to non-members before committing.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67743/new/

https://reviews.llvm.org/D67743



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


r372361 - [Consumed] Treat by-value class arguments as consuming by default, like rvalue refs.

2019-09-19 Thread Nicholas Allegra via cfe-commits
Author: comex
Date: Thu Sep 19 16:00:31 2019
New Revision: 372361

URL: http://llvm.org/viewvc/llvm-project?rev=372361&view=rev
Log:
[Consumed] Treat by-value class arguments as consuming by default, like rvalue 
refs.

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

Modified:
cfe/trunk/lib/Analysis/Consumed.cpp
cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp

Modified: cfe/trunk/lib/Analysis/Consumed.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Consumed.cpp?rev=372361&r1=372360&r2=372361&view=diff
==
--- cfe/trunk/lib/Analysis/Consumed.cpp (original)
+++ cfe/trunk/lib/Analysis/Consumed.cpp Thu Sep 19 16:00:31 2019
@@ -644,10 +644,10 @@ bool ConsumedStmtVisitor::handleCall(con
   continue;
 
 // Adjust state on the caller side.
-if (isRValueRef(ParamType))
-  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
-else if (ReturnTypestateAttr *RT = Param->getAttr())
+if (ReturnTypestateAttr *RT = Param->getAttr())
   setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
 else if (isPointerOrRef(ParamType) &&
  (!ParamType->getPointeeType().isConstQualified() ||
   isSetOnReadPtrType(ParamType)))

Modified: cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp?rev=372361&r1=372360&r2=372361&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp Thu Sep 19 16:00:31 2019
@@ -53,12 +53,18 @@ class CONSUMABLE(unconsumed) DestructorT
 public:
   DestructorTester();
   DestructorTester(int);
+  DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+  DestructorTester(DestructorTester &&);
   
   void operator*() CALLABLE_WHEN("unconsumed");
   
   ~DestructorTester() CALLABLE_WHEN("consumed");
+
 };
 
+void dtByVal(DestructorTester);
+void dtByValMarkUnconsumed(DestructorTester RETURN_TYPESTATE(unconsumed));
+
 void baf0(const ConsumableClass  var);
 void baf1(const ConsumableClass &var);
 void baf2(const ConsumableClass *var);
@@ -120,6 +126,19 @@ void testDestruction() {
  expected-warning {{invalid invocation of method 
'~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
 }
 
+void testDestructionByVal() {
+  {
+// both the var and the temporary are consumed:
+DestructorTester D0(nullptr);
+dtByVal((DestructorTester &&)D0);
+  }
+  {
+// the var is consumed but the temporary isn't:
+DestructorTester D1(nullptr);
+dtByValMarkUnconsumed((DestructorTester &&)D1); // expected-warning 
{{invalid invocation of method '~DestructorTester' on a temporary object while 
it is in the 'unconsumed' state}}
+  }
+}
+
 void testTempValue() {
   *ConsumableClass(); // expected-warning {{invalid invocation of method 
'operator*' on a temporary object while it is in the 'consumed' state}}
 }
@@ -413,10 +432,15 @@ void testParamReturnTypestateCallee(bool
   Param.consume();
 }
 
+void testRvalueRefParamReturnTypestateCallee(ConsumableClass &&Param 
RETURN_TYPESTATE(unconsumed)) {
+  Param.unconsume();
+}
+
 void testParamReturnTypestateCaller() {
   ConsumableClass var;
   
   testParamReturnTypestateCallee(true, var);
+  testRvalueRefParamReturnTypestateCallee((ConsumableClass &&)var);
   
   *var;
 }
@@ -480,6 +504,9 @@ void testCallingConventions() {
   
   baf2(&var);  
   *var;
+
+  baf3(var);
+  *var;
   
   baf4(var);  
   *var; // expected-warning {{invalid invocation of method 'operator*' on 
object 'var' while it is in the 'unknown' state}}


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


[PATCH] D67743: [Consumed] Treat by-value class arguments as consuming by default, like rvalue refs.

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372361: [Consumed] Treat by-value class arguments as 
consuming by default, like rvalue… (authored by comex, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67743?vs=220778&id=220916#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67743/new/

https://reviews.llvm.org/D67743

Files:
  cfe/trunk/lib/Analysis/Consumed.cpp
  cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp


Index: cfe/trunk/lib/Analysis/Consumed.cpp
===
--- cfe/trunk/lib/Analysis/Consumed.cpp
+++ cfe/trunk/lib/Analysis/Consumed.cpp
@@ -644,10 +644,10 @@
   continue;
 
 // Adjust state on the caller side.
-if (isRValueRef(ParamType))
-  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
-else if (ReturnTypestateAttr *RT = Param->getAttr())
+if (ReturnTypestateAttr *RT = Param->getAttr())
   setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
 else if (isPointerOrRef(ParamType) &&
  (!ParamType->getPointeeType().isConstQualified() ||
   isSetOnReadPtrType(ParamType)))
Index: cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
===
--- cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
+++ cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
@@ -53,12 +53,18 @@
 public:
   DestructorTester();
   DestructorTester(int);
+  DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+  DestructorTester(DestructorTester &&);
   
   void operator*() CALLABLE_WHEN("unconsumed");
   
   ~DestructorTester() CALLABLE_WHEN("consumed");
+
 };
 
+void dtByVal(DestructorTester);
+void dtByValMarkUnconsumed(DestructorTester RETURN_TYPESTATE(unconsumed));
+
 void baf0(const ConsumableClass  var);
 void baf1(const ConsumableClass &var);
 void baf2(const ConsumableClass *var);
@@ -120,6 +126,19 @@
  expected-warning {{invalid invocation of method 
'~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
 }
 
+void testDestructionByVal() {
+  {
+// both the var and the temporary are consumed:
+DestructorTester D0(nullptr);
+dtByVal((DestructorTester &&)D0);
+  }
+  {
+// the var is consumed but the temporary isn't:
+DestructorTester D1(nullptr);
+dtByValMarkUnconsumed((DestructorTester &&)D1); // expected-warning 
{{invalid invocation of method '~DestructorTester' on a temporary object while 
it is in the 'unconsumed' state}}
+  }
+}
+
 void testTempValue() {
   *ConsumableClass(); // expected-warning {{invalid invocation of method 
'operator*' on a temporary object while it is in the 'consumed' state}}
 }
@@ -413,10 +432,15 @@
   Param.consume();
 }
 
+void testRvalueRefParamReturnTypestateCallee(ConsumableClass &&Param 
RETURN_TYPESTATE(unconsumed)) {
+  Param.unconsume();
+}
+
 void testParamReturnTypestateCaller() {
   ConsumableClass var;
   
   testParamReturnTypestateCallee(true, var);
+  testRvalueRefParamReturnTypestateCallee((ConsumableClass &&)var);
   
   *var;
 }
@@ -480,6 +504,9 @@
   
   baf2(&var);  
   *var;
+
+  baf3(var);
+  *var;
   
   baf4(var);  
   *var; // expected-warning {{invalid invocation of method 'operator*' on 
object 'var' while it is in the 'unknown' state}}


Index: cfe/trunk/lib/Analysis/Consumed.cpp
===
--- cfe/trunk/lib/Analysis/Consumed.cpp
+++ cfe/trunk/lib/Analysis/Consumed.cpp
@@ -644,10 +644,10 @@
   continue;
 
 // Adjust state on the caller side.
-if (isRValueRef(ParamType))
-  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
-else if (ReturnTypestateAttr *RT = Param->getAttr())
+if (ReturnTypestateAttr *RT = Param->getAttr())
   setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+  setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
 else if (isPointerOrRef(ParamType) &&
  (!ParamType->getPointeeType().isConstQualified() ||
   isSetOnReadPtrType(ParamType)))
Index: cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
===
--- cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
+++ cfe/trunk/test/SemaCXX/warn-consumed-analysis.cpp
@@ -53,12 +53,18 @@
 public:
   DestructorTester();
   DestructorTester(int);
+  DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+  DestructorTester(DestructorTester &&);
   
   void operator*() CALLABLE_WHEN("unconsumed");
   
   ~DestructorTester() CALLABLE_WHEN("consumed");
+
 };
 
+void 

[PATCH] D67740: [Consumed] Refactor and improve diagnostics

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
comex marked 4 inline comments as done.
comex added a comment.

In D67740#1675564 , @dblaikie wrote:

> Looks like this might benefit from being split into independent changes - the 
> work related to templates (I haven't looked closely, but I assume that's 
> fairly indivisible) and the work related to other diagnostics seems fairly 
> separable - and maybe there's other pieces too.


Okay, I'll take the set_typestate/test_typestate part out of this and submit it 
separately.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3234
+  "consumed analysis attribute is attached to a "
+  "%select{static method|constructor}0 of class '%1'">,
+  InGroup, DefaultIgnore;

dblaikie wrote:
> "member function" would be more correct than "method" here (the diagnostics 
> in this file using the word "method" are mostly for Objective C things)
> 
> Are there other diagnostics that are similar? (like maybe function "const" - 
> which can't be on non-member, static member, or ctors - wouldn't that be the 
> same here? Where's the failure path for a non-member (free) function? Could 
> it be unified with the static member/ctor case you're adding?)
> 
> Ah, looks like in Attr.td these attributes could be flagged 
> "NonStaticCXXMethod" rather than "CXXMethod" - perhaps that narrower 
> classification wasn't available when this was originally implemented. (then, 
> I think, the attribute parsing logic will do the job of warning about the 
> misuse and ignoring the attribute entirely)
That doesn't check for constructors, but sure, I'll add a 
"NonStaticNonConstructorCXXMethod" instead (in the separate submission).




Comment at: lib/Sema/SemaDeclAttr.cpp:1237-1240
+else if (auto *CX = dyn_cast(X))
+  return CX->getThisType()->getPointeeType();
+else
+  return cast(X)->getCallResultType();

dblaikie wrote:
> Drop the else-after-return 
> https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
Done.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67740/new/

https://reviews.llvm.org/D67740



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


[PATCH] D67740: [Consumed] Refactor and improve diagnostics

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
comex updated this revision to Diff 220918.
comex marked 2 inline comments as done.
comex edited the summary of this revision.
comex added a comment.

Addressed feedback.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67740/new/

https://reviews.llvm.org/D67740

Files:
  include/clang/Analysis/Analyses/Consumed.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Analysis/Consumed.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/SemaCXX/warn-consumed-parsing.cpp

Index: test/SemaCXX/warn-consumed-parsing.cpp
===
--- test/SemaCXX/warn-consumed-parsing.cpp
+++ test/SemaCXX/warn-consumed-parsing.cpp
@@ -6,16 +6,6 @@
 #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
 #define TEST_TYPESTATE(state)   __attribute__ ((test_typestate(state)))
 
-// FIXME: This test is here because the warning is issued by the Consumed
-//analysis, not SemaDeclAttr.  The analysis won't run after an error
-//has been issued.  Once the attribute propagation for template
-//instantiation has been fixed, this can be moved somewhere else and the
-//definition can be removed.
-int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
-int returnTypestateForUnconsumable() {
-  return 0;
-}
-
 class AttrTester0 {
   void consumes()   __attribute__ ((set_typestate())); // expected-error {{'set_typestate' attribute takes one argument}}
   bool testUnconsumed() __attribute__ ((test_typestate())); // expected-error {{'test_typestate' attribute takes one argument}}
@@ -62,5 +52,33 @@
   Status {
 };
 
+int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{attribute 'return_typestate' invalid for return value of type 'int': expected a consumable class type as a value, reference, or rvalue reference}}
+int returnTypestateForUnconsumable() {
+  return 0;
+}
+
+struct CONSUMABLE(unknown) UselessAttrs {
+  void operator+(UselessAttrs) SET_TYPESTATE(consumed); // OK
+  template 
+  void a([[clang::param_typestate(consumed)]] const int &) {} // expected-warning {{attribute 'param_typestate' invalid for parameter of type 'const int &': expected a consumable class type as a value, reference, or rvalue reference}}
+  void b([[clang::return_typestate(consumed)]] UselessAttrs *) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'UselessAttrs *': expected a consumable class type as a value, reference, or rvalue reference}}
+  template 
+  void c([[clang::return_typestate(consumed)]] T) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'int': expected a consumable class type as a value, reference, or rvalue reference}}
+  template <> // test a template specialization
+  void c([[clang::return_typestate(consumed)]] long); // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'long': expected a consumable class type as a value, reference, or rvalue reference}}
+  void instantiate() {
+a(42);
+c(42);
+  }
+};
+void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to functions}}
 
+template 
+struct CONSUMABLE(unknown) ClassTemplateSpecialization;
+template 
+struct ClassTemplateSpecialization {
+  [[clang::return_typestate(consumed)]] ClassTemplateSpecialization(); // expected-warning {{attribute 'return_typestate' invalid for return value of type 'ClassTemplateSpecialization': expected a consumable class type as a value, reference, or rvalue reference}}
+  void a([[clang::return_typestate(consumed)]] T) {} // expected-warning {{attribute 'return_typestate' invalid for parameter of type 'long': expected a consumable class type as a value, reference, or rvalue reference}}
+};
+void testTS() { ClassTemplateSpecialization().a(1); } // expected-note {{in instantiation of template class 'ClassTemplateSpecialization' requested here}}
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -606,6 +606,11 @@
   continue;
 }
 
+if (isa(TmplAttr) || isa(TmplAttr)) {
+  if (!CheckParamOrReturnTypestateAttr(TmplAttr, New, Tmpl))
+continue; // don't add
+}
+
 assert(!TmplAttr->isPackExpansion());
 if (TmplAttr->isLateParsed() && LateAttrs) {
   // Late parsed attributes must be instantiated and attached after the
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1190,19 +1190,9 @@
 return;
   }
 
-  // FIXME: This check is curr

[PATCH] D67778: [Consumed] Narrow Subject for some attributes

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
comex created this revision.
comex added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is the former second part of https://reviews.llvm.org/D67740 that was 
split out (and rewritten).

Error out when the `callable_when`, `set_typestate` and `test_typestate` 
attributes are applied to a constructor or static method; previously Clang 
would ignore them or crash, depending on the attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D67778

Files:
  include/clang/Basic/Attr.td
  test/SemaCXX/warn-consumed-parsing.cpp


Index: test/SemaCXX/warn-consumed-parsing.cpp
===
--- test/SemaCXX/warn-consumed-parsing.cpp
+++ test/SemaCXX/warn-consumed-parsing.cpp
@@ -12,7 +12,7 @@
   void callableWhen()   __attribute__ ((callable_when())); // expected-error 
{{'callable_when' attribute takes at least 1 argument}}
 };
 
-int var0 SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' 
attribute only applies to functions}}
+int var0 SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' 
attribute only applies to non-static non-constructor member functions}}
 int var1 TEST_TYPESTATE(consumed); // expected-warning {{'test_typestate' 
attribute only applies to}}
 int var2 CALLABLE_WHEN("consumed"); // expected-warning {{'callable_when' 
attribute only applies to}}
 int var3 CONSUMABLE(consumed); // expected-warning {{'consumable' attribute 
only applies to classes}}
@@ -58,7 +58,11 @@
 }
 
 struct CONSUMABLE(unknown) UselessAttrs {
+  static void x() SET_TYPESTATE(consumed); // expected-warning 
{{'set_typestate' attribute only applies to non-static non-constructor member 
functions}}
+  UselessAttrs() SET_TYPESTATE(consumed); // expected-warning 
{{'set_typestate' attribute only applies to non-static non-constructor member 
functions}}
+  UselessAttrs(int) CALLABLE_WHEN(consumed); // expected-warning 
{{'callable_when' attribute only applies to non-static non-constructor member 
functions}}
   void operator+(UselessAttrs) SET_TYPESTATE(consumed); // OK
+  static void *operator new(unsigned long) SET_TYPESTATE(consumed); // 
expected-warning {{'set_typestate' attribute only applies to non-static 
non-constructor member functions}}
   template 
   void a([[clang::param_typestate(consumed)]] const int &) {} // 
expected-warning {{attribute 'param_typestate' invalid for parameter of type 
'const int &': expected a consumable class type as a value, reference, or 
rvalue reference}}
   void b([[clang::return_typestate(consumed)]] UselessAttrs *) {} // 
expected-warning {{attribute 'return_typestate' invalid for parameter of type 
'UselessAttrs *': expected a consumable class type as a value, reference, or 
rvalue reference}}
@@ -71,7 +75,7 @@
 c(42);
   }
 };
-void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // 
expected-warning {{'set_typestate' attribute only applies to functions}}
+void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // 
expected-warning {{'set_typestate' attribute only applies to non-static 
non-constructor member functions}}
 
 template 
 struct CONSUMABLE(unknown) ClassTemplateSpecialization;
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -96,6 +96,11 @@
[{!S->isStatic()}],
"non-static member functions">;
 
+def NonStaticNonConstructorCXXMethod
+: SubsetSubjectisStatic() && !isa(S)}],
+"non-static non-constructor member functions">;
+
 def NonStaticNonConstCXXMethod
 : SubsetSubjectisStatic() && !S->isConst()}],
@@ -2725,7 +2730,7 @@
   // to C++ function (but doesn't require it to be a member function).
   // FIXME: should this attribute have a CPlusPlus language option?
   let Spellings = [Clang<"callable_when", 0>];
-  let Subjects = SubjectList<[CXXMethod]>;
+  let Subjects = SubjectList<[NonStaticNonConstructorCXXMethod]>;
   let Args = [VariadicEnumArgument<"CallableStates", "ConsumedState",
["unknown", "consumed", "unconsumed"],
["Unknown", "Consumed", "Unconsumed"]>];
@@ -2761,7 +2766,7 @@
   // to C++ function (but doesn't require it to be a member function).
   // FIXME: should this attribute have a CPlusPlus language option?
   let Spellings = [Clang<"set_typestate", 0>];
-  let Subjects = SubjectList<[CXXMethod]>;
+  let Subjects = SubjectList<[NonStaticNonConstructorCXXMethod]>;
   let Args = [EnumArgument<"NewState", "ConsumedState",
["unknown", "consumed", "unconsumed"],
["Unknown", "Consumed", "Unconsumed"]>];
@@ -2773,7 +2778,7 @@
   // to C++ function (but doesn't require it to be a member function).
   // FIXME: should this attribute have a CPlusPlus language

r372363 - [NFCI] Always initialize const members of AttributeCommonInfo

2019-09-19 Thread Alex Langford via cfe-commits
Author: xiaobai
Date: Thu Sep 19 17:16:32 2019
New Revision: 372363

URL: http://llvm.org/viewvc/llvm-project?rev=372363&view=rev
Log:
[NFCI] Always initialize const members of AttributeCommonInfo

Some compilers require that const fields of an object must be explicitly
initialized by the constructor. I ran into this issue building with
clang 3.8 on Ubuntu 16.04.

Modified:
cfe/trunk/include/clang/Basic/AttributeCommonInfo.h

Modified: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttributeCommonInfo.h?rev=372363&r1=372362&r2=372363&view=diff
==
--- cfe/trunk/include/clang/Basic/AttributeCommonInfo.h (original)
+++ cfe/trunk/include/clang/Basic/AttributeCommonInfo.h Thu Sep 19 17:16:32 2019
@@ -74,11 +74,11 @@ protected:
 
 public:
   AttributeCommonInfo(SourceRange AttrRange)
-  : AttrRange(AttrRange), AttrKind(0), SyntaxUsed(0),
+  : AttrRange(AttrRange), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
 SpellingIndex(SpellingNotCalculated) {}
 
   AttributeCommonInfo(SourceLocation AttrLoc)
-  : AttrRange(AttrLoc), AttrKind(0), SyntaxUsed(0),
+  : AttrRange(AttrLoc), ScopeLoc(), AttrKind(0), SyntaxUsed(0),
 SpellingIndex(SpellingNotCalculated) {}
 
   AttributeCommonInfo(const IdentifierInfo *AttrName,


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


[PATCH] D67778: [Consumed] Narrow Subject for some attributes

2019-09-19 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - thanks!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67778/new/

https://reviews.llvm.org/D67778



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


[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

2019-09-19 Thread Nicholas Allegra via Phabricator via cfe-commits
comex updated this revision to Diff 220926.
comex added a comment.

In D67647#1674795 , @dblaikie wrote:

> Right - I was suggesting that could be changed. Would it be OK to you to 
> change arguments() to return ArrayRef? Or would you rather avoid that to 
> avoid baking in more dependence on that alias violation?


I'd rather avoid that for the reason you said.

> Same reason I've pushed back on adding things like "empty()" or "size()", 
> etc. Which are fine (& have been added in STLExtras) as free functions.

Okay, I'll just do that then.  Added as `index(Range, N)`, matching range-v3 
(though not the actual C++20 draft).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67647/new/

https://reviews.llvm.org/D67647

Files:
  clang/include/clang/AST/Stmt.h
  clang/lib/Analysis/Consumed.cpp
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/iterator_range.h

Index: llvm/include/llvm/ADT/iterator_range.h
===
--- llvm/include/llvm/ADT/iterator_range.h
+++ llvm/include/llvm/ADT/iterator_range.h
@@ -20,9 +20,17 @@
 
 #include 
 #include 
+#include 
 
 namespace llvm {
 
+template 
+constexpr bool is_random_iterator() {
+  return std::is_same<
+typename std::iterator_traits::iterator_category,
+std::random_access_iterator_tag>::value;
+}
+
 /// A range adaptor for a pair of iterators.
 ///
 /// This just wraps two iterators into a range-compatible interface. Nothing
@@ -58,11 +66,31 @@
   return iterator_range(std::move(p.first), std::move(p.second));
 }
 
+/// Non-random-iterator version
 template 
-iterator_range()))> drop_begin(T &&t,
-  int n) {
-  return make_range(std::next(adl_begin(t), n), adl_end(t));
+auto drop_begin(T &&t, int n) ->
+  typename std::enable_if(),
+  iterator_range>::type {
+  auto begin = adl_begin(t);
+  auto end = adl_end(t);
+  for (int i = 0; i < n; i++) {
+assert(begin != end);
+++begin;
+  }
+  return make_range(begin, end);
 }
+
+/// Optimized version for random iterators
+template 
+auto drop_begin(T &&t, int n) ->
+  typename std::enable_if(),
+  iterator_range>::type {
+  auto begin = adl_begin(t);
+  auto end = adl_end(t);
+  assert(end - begin >= n && "Dropping more elements than exist!");
+  return make_range(std::next(begin, n), end);
+}
+
 }
 
 #endif
Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1573,6 +1573,14 @@
 }
 template  constexpr T *to_address(T *P) { return P; }
 
+template 
+auto index(R &&TheRange,
+  typename std::iterator_traits>::difference_type N)
+  -> decltype(TheRange.begin()[N]) {
+  assert(N < TheRange.end() - TheRange.begin() && "Index out of range!");
+  return TheRange.begin()[N];
+}
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_STLEXTRAS_H
Index: clang/lib/Analysis/Consumed.cpp
===
--- clang/lib/Analysis/Consumed.cpp
+++ clang/lib/Analysis/Consumed.cpp
@@ -494,8 +494,10 @@
   void checkCallability(const PropagationInfo &PInfo,
 const FunctionDecl *FunDecl,
 SourceLocation BlameLoc);
-  bool handleCall(const CallExpr *Call, const Expr *ObjArg,
-  const FunctionDecl *FunD);
+
+  using ArgRange = llvm::iterator_range;
+  bool handleCall(const Expr *Call, const Expr *ObjArg,
+  ArgRange args, const FunctionDecl *FunD);
 
   void VisitBinaryOperator(const BinaryOperator *BinOp);
   void VisitCallExpr(const CallExpr *Call);
@@ -608,22 +610,21 @@
 // Factors out common behavior for function, method, and operator calls.
 // Check parameters and set parameter state if necessary.
 // Returns true if the state of ObjArg is set, or false otherwise.
-bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg,
+bool ConsumedStmtVisitor::handleCall(const Expr *Call,
+ const Expr *ObjArg,
+ ArgRange Args,
  const FunctionDecl *FunD) {
-  unsigned Offset = 0;
-  if (isa(Call) && isa(FunD))
-Offset = 1;  // first argument is 'this'
-
   // check explicit parameters
-  for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
+  unsigned Index = 0;
+  for (const Expr *Arg : Args) {
 // Skip variable argument lists.
-if (Index - Offset >= FunD->getNumParams())
+if (Index >= FunD->getNumParams())
   break;
 
-const ParmVarDecl *Param = FunD->getParamDecl(Index - Offset);
+const ParmVarDecl *Param = FunD->getParamDecl(Index++);
 QualType ParamType = Param->getType();
 
-InfoEntry Entry = findInfo(Call->getArg(Index));
+InfoEntry Entry = findInfo(Arg);
 
 if (Entry == P

[PATCH] D67052: Add reference type transformation builtins

2019-09-19 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver marked 5 inline comments as done.
zoecarver added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:1612
 break;
+  case DeclSpec::TST_addLValueReferenceType:
+  case DeclSpec::TST_addRValueReferenceType:

EricWF wrote:
> This should be merged with the above case. 
The reason I haven't done that is because `__underlying_type` will default to 
`IntTy` if it fails. We don't want that for the others (maybe we don't want 
that here either?). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



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


[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

2019-09-19 Thread Stephane Moore via Phabricator via cfe-commits
stephanemoore marked 3 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp:56
+  const auto *ID = Result.Nodes.getNodeAs("impl");
+  diag(ID->getLocation(), "%0 implements -isEqual: without implementing -hash")
+  << ID;

aaron.ballman wrote:
> Do you think we could generate a fixit to add the `hash` method? Do you think 
> we could even add a default implementation that returns the pointer to the 
> object (assuming that's the correct default behavior)?
> Do you think we could generate a fixit to add the hash method?

I think it would be pretty tough to generate a reasonable hash method without 
knowing the equality and hashing semantics that the scenario calls for.

Here is an analogous situation presented in C++ (please excuse the hastily 
assembled sample code):
```
namespace {

class NSObject {
  public:
NSObject() {}
virtual ~NSObject() {}

virtual bool isEqual(const NSObject *other) const {
  return this == other;
}
virtual unsigned long long hash() const {
  return (unsigned long long)this;
}
};

}

#include 
#include 

namespace {

class Movie : public virtual NSObject {
  private:
std::string name;
std::string language;

  public:
Movie(std::string name, std::string language) : name(name), 
language(language) {}
~Movie() override {}
bool isEqual(const NSObject *other) const override {
  if (auto otherMovie = dynamic_cast(other)) {
// Movies with the same name are considered equal
// regardless of the language of the screening.
return name == otherMovie->name;
  }
  return false;
}
unsigned long long hash() const override {
  return name.length();
}
};

}
```

As before, the base class uses pointer equality and the pointer as a hash. A 
subclass may arbitrarily add additional state but only the developer knows 
which added state factors into equality operations and consequently should be 
considered—but not necessarily required—in the hash operation. The matter can 
technically get even more complicated if an object stores state externally. I 
would hope that externally stored state would not factor into the equality 
operation of an object but I hesitate to make an assumption.

The developer is also in the best position to prioritize different properties 
of the hash function including performance, collision resistance, uniformity, 
and non-invertibility.

Writing effective hash functions is probably difficult independent of the 
programming language but it might help to consider some specific examples in 
Objective-C. 
[GPBMessage](https://github.com/protocolbuffers/protobuf/blob/ffa6bfc/objectivec/GPBMessage.m),
 the Objective-C base class for Google Protocol Buffer message classes, 
implements `-hash` but has an [extensive 
comment](https://github.com/protocolbuffers/protobuf/blob/ffa6bfc/objectivec/GPBMessage.m#L2749)
 explaining that its complex but generic implementation is not generally 
optimal and recommends that developers override `-hash` and `-isEqual:` to 
optimize for runtime performance. In contrast, the basic collection classes in 
Apple's Foundation framework have [surprisingly simple hash 
behavior](https://github.com/stephanemoore/archives/blob/master/objc/tips/hashing-basic-collections.md)
 that clearly indicate priority to runtime performance over uniformity and 
collision resistance. The former is a conservatively expensive hash function 
and the latter is a conservatively inexpensive hash function.

> Do you think we could even add a default implementation that returns the 
> pointer to the object (assuming that's the correct default behavior)?

A hash returning the object pointer is already inherited from the superclass 
(i..e, `-[NSObject hash]`). Defining an override that returns the object 
pointer would be a functional no-op for classes directly derived from 
`NSObject` (although the explicit override could be useful as a signal of 
intended behavior).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67737/new/

https://reviews.llvm.org/D67737



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


[PATCH] D49091: Warn about usage of __has_include/__has_include_next in macro expansions

2019-09-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Under http://eel.is/c++draft/cpp#cond-7.sentence-2, the identifier 
`__has_include` can't appear anywhere other than in the context of an `#if`, 
`#elif`, `#iifdef`, or `#ifndef`. That's what we should be checking for and 
diagnosing here (and we should produce an `ExtWarn` rather than a `Warning` for 
this case, because such code is ill-formed, and accepting it at all is a 
language extension compared to the C++ rules). We should apply the same 
behavior to `__has_cpp_attribute`, to which the same rule applies.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49091/new/

https://reviews.llvm.org/D49091



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


[PATCH] D67787: Add 8548 CPU definition and attributes

2019-09-19 Thread Justin Hibbits via Phabricator via cfe-commits
jhibbits added a comment.
Herald added a subscriber: wuzish.

I made 8548 an alias in clang to e500, because e500 is recognized in llvm as a 
CPU, so gets us the feature list and, more importantly, the instruction 
scheduler.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67787/new/

https://reviews.llvm.org/D67787



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


[PATCH] D67787: Add 8548 CPU definition and attributes

2019-09-19 Thread Justin Hibbits via Phabricator via cfe-commits
jhibbits created this revision.
jhibbits added reviewers: nemanjai, hfinkel, joerg, kthomsen.
Herald added subscribers: cfe-commits, jsji, MaskRay, kbarton.
Herald added a project: clang.

8548 CPU is GCC's name for the e500v2, so accept this in clang.  The
e500v2 doesn't support lwsync, so define __NO_LWSYNC__ for this as well,
as GCC does.


Repository:
  rC Clang

https://reviews.llvm.org/D67787

Files:
  lib/Basic/Targets/PPC.cpp
  lib/Basic/Targets/PPC.h
  lib/Driver/ToolChains/Arch/PPC.cpp
  test/Misc/target-invalid-cpu-note.c
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -7584,6 +7584,10 @@
 //
 // PPC32-SPE:#define __NO_FPRS__ 1
 // PPC32-SPE:#define __SPE__ 1
+// 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -target-cpu 8548 < /dev/null | FileCheck -match-full-lines -check-prefix PPC8548 %s
+//
+// PPC8548:#define __NO_LWSYNC__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-apple-darwin8 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-DARWIN %s
 //
Index: test/Misc/target-invalid-cpu-note.c
===
--- test/Misc/target-invalid-cpu-note.c
+++ test/Misc/target-invalid-cpu-note.c
@@ -79,10 +79,10 @@
 // PPC: error: unknown target CPU 'not-a-cpu'
 // PPC: note: valid target CPU values are: generic, 440, 450, 601, 602, 603,
 // PPC-SAME: 603e, 603ev, 604, 604e, 620, 630, g3, 7400, g4, 7450, g4+, 750,
-// PPC-SAME: 970, g5, a2, a2q, e500mc, e5500, power3, pwr3, power4, pwr4,
-// PPC-SAME: power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x, power7,
-// PPC-SAME: pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64, ppc64,
-// PPC-SAME: powerpc64le, ppc64le
+// PPC-SAME: 8548, 970, g5, a2, a2q, e500, e500mc, e5500, power3, pwr3, power4,
+// PPC-SAME: pwr4, power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x,
+// PPC-SAME: power7, pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64,
+// PPC-SAME: ppc64, powerpc64le, ppc64le
 
 // RUN: not %clang_cc1 -triple mips--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix MIPS
 // MIPS: error: unknown target CPU 'not-a-cpu'
Index: lib/Driver/ToolChains/Arch/PPC.cpp
===
--- lib/Driver/ToolChains/Arch/PPC.cpp
+++ lib/Driver/ToolChains/Arch/PPC.cpp
@@ -52,6 +52,7 @@
 .Case("7450", "7450")
 .Case("G4+", "g4+")
 .Case("750", "750")
+.Case("8548", "e500")
 .Case("970", "970")
 .Case("G5", "g5")
 .Case("a2", "a2")
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -44,7 +44,8 @@
 ArchDefinePwr8 = 1 << 12,
 ArchDefinePwr9 = 1 << 13,
 ArchDefineA2 = 1 << 14,
-ArchDefineA2q = 1 << 15
+ArchDefineA2q = 1 << 15,
+ArchDefineE500 = 1 << 16
   } ArchDefineTypes;
 
 
@@ -145,6 +146,7 @@
  ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
  ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
  ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+  .Case("8548", ArchDefineE500)
   .Default(ArchDefineNone);
 }
 return CPUKnown;
Index: lib/Basic/Targets/PPC.cpp
===
--- lib/Basic/Targets/PPC.cpp
+++ lib/Basic/Targets/PPC.cpp
@@ -157,6 +157,8 @@
 Builder.defineMacro("_ARCH_A2Q");
 Builder.defineMacro("_ARCH_QP");
   }
+  if (ArchDefs & ArchDefineE500)
+Builder.defineMacro("__NO_LWSYNC__");
 
   if (getTriple().getVendor() == llvm::Triple::BGQ) {
 Builder.defineMacro("__bg__");
@@ -312,6 +314,8 @@
 .Case("pwr8", true)
 .Default(false);
 
+  Features["spe"] = (CPU == "e500");
+
   if (!ppcUserFeaturesCheck(Diags, FeaturesVec))
 return false;
 
@@ -449,16 +453,16 @@
 }
 
 static constexpr llvm::StringLiteral ValidCPUNames[] = {
-{"generic"}, {"440"}, {"450"}, {"601"},{"602"},
-{"603"}, {"603e"},{"603ev"},   {"604"},{"604e"},
-{"620"}, {"630"}, {"g3"},  {"7400"},   {"g4"},
-{"7450"},{"g4+"}, {"750"}, {"970"},{"g5"},
-{"a2"},  {"a2q"}, {"e500mc"},  {"e5500"},  {"power3"},
-{"pwr3"},{"power4"},  {"pwr4"},{"power5"}, {"pwr5"},
-{"power5x"}, {"pwr5x"},   {"power6"},  {"pwr6"},   {"power6x"},
-{"pwr6x"},   {"power7"},  {"pwr7"},{"power8"}, {"pwr8"},
-{"power9"},  {"pwr9"},{"powerpc"}, {"ppc"},{"powerpc64"},
-{"ppc64"},   {"powerpc64le"}, {"ppc64le"},
+{"generic"},   {"440"},   {"450"}, {"601"}, {"602"},
+{"603"},   {"603e"}, 

r372368 - Finish building the full-expression for a static_assert expression

2019-09-19 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Sep 19 20:29:19 2019
New Revision: 372368

URL: http://llvm.org/viewvc/llvm-project?rev=372368&view=rev
Log:
Finish building the full-expression for a static_assert expression
before evaluating it rather than afterwards.

This is groundwork for C++20's P0784R7, where non-trivial destructors
can be constexpr, so we need ExprWithCleanups markers in constant
expressions.

No significant functionality change intended (though this fixes a bug
only visible through libclang / -ast-dump / tooling: we now store the
converted condition on the StaticAssertDecl rather than the original).

Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/Index/Core/index-source.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=372368&r1=372367&r2=372368&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 19 20:29:19 2019
@@ -14182,8 +14182,17 @@ Decl *Sema::BuildStaticAssertDeclaration
 if (Converted.isInvalid())
   Failed = true;
 
+ExprResult FullAssertExpr =
+ActOnFinishFullExpr(Converted.get(), StaticAssertLoc,
+/*DiscardedValue*/ false,
+/*IsConstexpr*/ true);
+if (FullAssertExpr.isInvalid())
+  Failed = true;
+else
+  AssertExpr = FullAssertExpr.get();
+
 llvm::APSInt Cond;
-if (!Failed && VerifyIntegerConstantExpression(Converted.get(), &Cond,
+if (!Failed && VerifyIntegerConstantExpression(AssertExpr, &Cond,
   diag::err_static_assert_expression_is_not_constant,
   /*AllowFold=*/false).isInvalid())
   Failed = true;
@@ -14209,16 +14218,16 @@ Decl *Sema::BuildStaticAssertDeclaration
   }
   Failed = true;
 }
+  } else {
+ExprResult FullAssertExpr = ActOnFinishFullExpr(AssertExpr, 
StaticAssertLoc,
+/*DiscardedValue*/false,
+/*IsConstexpr*/true);
+if (FullAssertExpr.isInvalid())
+  Failed = true;
+else
+  AssertExpr = FullAssertExpr.get();
   }
 
-  ExprResult FullAssertExpr = ActOnFinishFullExpr(AssertExpr, StaticAssertLoc,
-  /*DiscardedValue*/false,
-  /*IsConstexpr*/true);
-  if (FullAssertExpr.isInvalid())
-Failed = true;
-  else
-AssertExpr = FullAssertExpr.get();
-
   Decl *Decl = StaticAssertDecl::Create(Context, CurContext, StaticAssertLoc,
 AssertExpr, AssertMessage, RParenLoc,
 Failed);

Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=372368&r1=372367&r2=372368&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Thu Sep 19 20:29:19 2019
@@ -461,12 +461,12 @@ struct StaticAssertRef {
 };
 
 static_assert(StaticAssertRef::constVar, "index static asserts");
-// CHECK: [[@LINE-1]]:32 | static-property/C++ | constVar | 
c:@S@StaticAssertRef@constVar | __ZN15StaticAssertRef8constVarE | Ref | rel: 0
+// CHECK: [[@LINE-1]]:32 | static-property/C++ | constVar | 
c:@S@StaticAssertRef@constVar | __ZN15StaticAssertRef8constVarE | Ref,Read | 
rel: 0
 // CHECK: [[@LINE-2]]:15 | struct/C++ | StaticAssertRef | c:@S@StaticAssertRef 
|  | Ref | rel: 0
 
 void staticAssertInFn() {
   static_assert(StaticAssertRef::constVar, "index static asserts");
-// CHECK: [[@LINE-1]]:34 | static-property/C++ | constVar | 
c:@S@StaticAssertRef@constVar | __ZN15StaticAssertRef8constVarE | Ref,RelCont | 
rel: 1
+// CHECK: [[@LINE-1]]:34 | static-property/C++ | constVar | 
c:@S@StaticAssertRef@constVar | __ZN15StaticAssertRef8constVarE | 
Ref,Read,RelCont | rel: 1
 // CHECK-NEXT: RelCont | staticAssertInFn | c:@F@staticAssertInFn#
 // CHECK: [[@LINE-3]]:17 | struct/C++ | StaticAssertRef | c:@S@StaticAssertRef 
|  | Ref,RelCont | rel: 1
 // CHECK-NEXT: RelCont | staticAssertInFn | c:@F@staticAssertInFn#


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