[PATCH] D50102: [clang-tidy] Use ExprMutationAnalyzer in performance-unnecessary-value-param

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:108
 return;
+  if (const auto *Ctor = dyn_cast(Function)) {
+for (const auto *Init : Ctor->inits()) {

shuaiwang wrote:
> hokein wrote:
> > Is this a new fix or  a special case not being caught by 
> > `ExprMutationAnalyzer`?  Do we have a test case covered it?
> It's a special case.
> ExprMutationAnalyzer is designed to analyze within a "scope" indicated by a 
> Stmt. So when trying to analyze a "function", merely looking at function body 
> is not sufficient because CXXCtorInitializer is not part of the function 
> body. So far we only care about this special case when trying to figure out 
> whether a function param is mutated or not, which is exactly what this check 
> is doing. We might consider making this part of ExprMutationAnalyzer but IMO 
> that depends on whether there are more use cases of this special case in the 
> future because we'll need some tricks there to match different types of top 
> level nodes.
> 
> This is already captured in existing unit test case, which basically looks 
> like this:
> ```
> class Foo {
> public:
>   Foo(ExpensiveObj x) : x(std::move(x)) {}
> private:
>   ExpensiveObj x;
> };
> ```
Thanks for the detailed clarification! That makes sense. Could you also add a 
proper comment in the code?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50102



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

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 158941.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D50189

Files:
  lib/Tooling/Core/Lookup.cpp
  unittests/Tooling/LookupTest.cpp

Index: unittests/Tooling/LookupTest.cpp
===
--- unittests/Tooling/LookupTest.cpp
+++ unittests/Tooling/LookupTest.cpp
@@ -68,7 +68,7 @@
   "namespace b { void f() { a::foo(); } }\n");
 
   Visitor.OnCall = [&](CallExpr *Expr) {
-EXPECT_EQ("a::bar", replaceCallExpr(Expr, "::a::bar"));
+EXPECT_EQ("::a::bar", replaceCallExpr(Expr, "::a::bar"));
   };
   Visitor.runOver("namespace a { void foo(); }\n"
   "namespace b { namespace a { void foo(); }\n"
@@ -127,6 +127,38 @@
   "namespace a { namespace b { namespace c {"
   "void f() { foo(); }"
   "} } }\n");
+
+  // If the shortest name is ambiguous, we need to add more qualifiers.
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("::a::y::bar", replaceCallExpr(Expr, "::a::y::bar"));
+  };
+  Visitor.runOver(R"(
+namespace a {
+namespace b {
+namespace x { void foo() {} }
+namespace y { void foo() {} }
+}
+}
+
+namespace a {
+namespace b {
+void f() { x::foo(); }
+}
+})");
+
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("y::bar", replaceCallExpr(Expr, "::y::bar"));
+  };
+  Visitor.runOver(R"(
+namespace a {
+namespace b {
+namespace x { void foo() {} }
+namespace y { void foo() {} }
+}
+}
+
+void f() { a::b::x::foo(); }
+)");
 }
 
 TEST(LookupTest, replaceNestedClassName) {
Index: lib/Tooling/Core/Lookup.cpp
===
--- lib/Tooling/Core/Lookup.cpp
+++ lib/Tooling/Core/Lookup.cpp
@@ -14,6 +14,7 @@
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,6 +115,37 @@
   return false;
 }
 
+// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
+// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
+// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
+// can be resolved to ::a::y::bar, which can cause compile error.
+// FIXME: consider using namespaces.
+static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
+   const DeclContext &UseContext) {
+  assert(QName.startswith("::"));
+  if (Spelling.startswith("::"))
+return false;
+
+  // Lookup the first component of Spelling in all enclosing namespaces and
+  // check if there is any existing symbols with the same name but in different
+  // scope.
+  StringRef Head = Spelling.split("::").first;
+
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(&UseContext);
+  auto &AST = UseContext.getParentASTContext();
+  StringRef TrimmedQName = QName.substr(2);
+  for (const auto *NS : UseNamespaces) {
+auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+if (!LookupRes.empty()) {
+  for (const NamedDecl *Res : LookupRes)
+if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+  return true;
+}
+  }
+  return false;
+}
+
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
const DeclContext *UseContext,
const NamedDecl *FromDecl,
@@ -146,6 +178,14 @@
   // figure out how good a namespace match we have with our destination type.
   // We work backwards (from most specific possible namespace to least
   // specific).
-  return getBestNamespaceSubstr(UseContext, ReplacementString,
-isFullyQualified(Use));
+  StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,
+   isFullyQualified(Use));
+  // Use the fully qualified name if the suggested name is ambiguous.
+  // FIXME: consider re-shortening the name until the name is not ambiguous. We
+  // are not doing this because ambiguity is pretty bad and we should not try to
+  // be clever in handling such cases. Making this noticeable to users seems to
+  // be a better option.
+  return isAmbiguousNameInScope(Suggested, ReplacementString, *UseContext)
+ ? ReplacementString
+ : Suggested;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Tooling/Core/Lookup.cpp:139
+const NamespaceDecl *NS = *I;
+auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+if (!LookupRes.empty()) {

ilya-biryukov wrote:
> This will not take using namespaces into account, right? Do we care about 
> those or is this a known limitation of the tools?
Right, I think we don't handle using namespaces in this library due to 
limitation/importance etc. Added a FIXME.


Repository:
  rC Clang

https://reviews.llvm.org/D50189



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

[PATCH] D50193: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the change! Overall LG, mostly NITs about the tests.




Comment at: clangd/CodeComplete.cpp:959
   bool Incomplete = false; // Would more be available with a higher limit?
-  llvm::Optional Filter;   // Initialized once Sema runs.
-  std::vector QueryScopes;  // Initialized once Sema runs.
+  llvm::Optional Filter;  // Initialized once Sema runs.
+  std::vector QueryScopes; // Initialized once Sema runs.

NIT: this change is unrelated, maybe remove from the diff?

We can submit it separately without review, since it's a noop formatting cleanup



Comment at: clangd/CodeComplete.cpp:1101
   // The bundles are scored and top results are returned, best to worst.
-  std::vector
-  mergeResults(const std::vector &SemaResults,
-   const SymbolSlab &IndexResults) {
+  std::vector mergeResults(const SymbolSlab &IndexResults) {
 trace::Span Tracer("Merge and score results");

Same here: maybe remove from this change and submit separately? 
Again, no review needed, this is clearly a no-op change



Comment at: clangd/CodeComplete.cpp:1288
+  LSP.additionalTextEdits.reserve(FixIts.size() + (HeaderInsertion ? 1 : 0));
+  for (const auto &FixIt : FixIts)
+LSP.additionalTextEdits.push_back(FixIt);

IIUC, it does not play nicely with the insertText in VSCode, right? (I.e. 
completing `this.^` will produce `this-push_back`)
Let's add a `FIXME` comment explaining this case and indicate how we're going 
to fix this (using `textEdit` instead of the `insertText`, right?)



Comment at: unittests/clangd/CodeCompleteTests.cpp:480
   auto Results = completions("int main() { abs^ }", {ns("absl"), 
func("absb")});
-  EXPECT_THAT(Results.Completions, HasSubsequence(Named("absb"), 
Named("absl")));
+  EXPECT_THAT(Results.Completions,
+  HasSubsequence(Named("absb"), Named("absl")));

NIT: also submit as a separate change?



Comment at: unittests/clangd/CodeCompleteTests.cpp:1354
+  void MemberFunction();
+  Auxilary* operator->() const { return Aux; }
+ private:

Maybe remove the body of `opeartor->()` and the private field?
Will make the test a little smaller.



Comment at: unittests/clangd/CodeCompleteTests.cpp:1358
+};
+namespace ns {
+  void f() {

Maybe remove `namespace` and put everything into global ns? 



Comment at: unittests/clangd/CodeCompleteTests.cpp:1361
+ClassWithPtr x;
+x->MemberFunction^;
+  }

Maybe tests without the prefix, so that both AuxFunction and MemberFunction are 
available and assert that AuxFunction does not have a fix-it?



Comment at: unittests/clangd/CodeCompleteTests.cpp:1369
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range.start.line = 15;
+  ReplacementEdit.range.start.character = 13;

This can be made more readable with helpers from 
`unittests/clangd/Annotations.h`.

```
Annotations Source(R"cpp(
/// 
void f() {
  ClassWithPtr x;
  x[[->]]MemberFunction^ 
})cpp");

Source.point(); // <-- completion point
Source.range(); // <-- range for the fix-it
```

Our `completions` helper already has parsing of annotations internally, though. 
But we can write a new overload for the helper that accepts completion point 
and the parsed text directly.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50193



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

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Eli, thanks for the feedback.

> Yes, this logic should be in TargetParser, not here. Trying to rewrite the 
> target features afterwards is messy at best. (Actually, the target feature 
> list generated by TargetParser probably shouldn't contain the string "crypto" 
> at all.)

I appreciate there is room for improvement here, which is an understatement! :) 
I probably should have mentioned earlier that my colleague is working on 
targetparser and options, and he will send the proposal in the form of an RFC 
to the dev list soon. Very briefly, the proposal will elaborate on how we want 
to capture/enforce architecture extension dependencies (I believe thus also 
disallow architecturally invalid combinations), imply options, and e.g. warn on 
redundant options.

I want to move the crypto logic to this new framework as soon it is there. 
Thus, for the time being, this is a stopgap to demonstrate what we want to 
achieve (with crypto), and also quite importantly, we have something that works 
today. But again, I fully agree that the current implementation is far from 
ideal, but hopefully with these explanations is somewhat acceptable.


https://reviews.llvm.org/D50179



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

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D50189



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

[PATCH] D50229: +fp16fml feature for ARM and AArch64

[Bernard Ogden via Phabricator via cfe-commits]

bogden created this revision.
bogden added a reviewer: SjoerdMeijer.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, chrib, kristof.beyls.

v8.4-A adds a few fp16 instructions that can optionally be implemented
in CPUs of v8.2-A and above.

This patch adds a feature to clang to permit selection of these
instructions. This interacts with the +fp16 option as follows:

1. Prior to v8.4-A

+fp16fml implies +fp16
+nofp16 implies +nofp16fml

2. From v8.4-A

The above conditions apply, additionally:
+fp16 implies +fp16fml


Repository:
  rC Clang

https://reviews.llvm.org/D50229

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c
  test/Preprocessor/aarch64-target-features.c
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -21,18 +21,58 @@
 // CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1
 // CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
 
-// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xe
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
 
-// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// +fp16fml without neon doesn't make sense as the fp16fml instructions all require SIMD.
+// However, as +fp16fml implies +fp16 there is a set of defines that we would expect.
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
 // CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xe
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
-//
+
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FU

[PATCH] D50230: clang-format: [JS] don't break comments before any '{'

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: krasimir.

Previously, clang-format would avoid breaking before the first `{`
found, but then happily break before subsequent '{'s on the line. This
change fixes that by looking for the first location that has no opening
curly, if any.


Repository:
  rC Clang

https://reviews.llvm.org/D50230

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


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -99,7 +99,7 @@
   // In JavaScript, some @tags can be followed by {, and machinery that parses
   // these comments will fail to understand the comment if followed by a line
   // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
+  while (Style.Language == FormatStyle::LK_JavaScript &&
   SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
   Text[SpaceOffset + 1] == '{')
 SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -99,7 +99,7 @@
   // In JavaScript, some @tags can be followed by {, and machinery that parses
   // these comments will fail to understand the comment if followed by a line
   // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
+  while (Style.Language == FormatStyle::LK_JavaScript &&
   SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
   Text[SpaceOffset + 1] == '{')
 SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50230: clang-format: [JS] don't break comments before any '{'

[Martin Probst via Phabricator via cfe-commits]

mprobst updated this revision to Diff 158946.
mprobst added a comment.

Also handle multiple numbered list tokens.


Repository:
  rC Clang

https://reviews.llvm.org/D50230

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


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,19 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,19 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be interpreted
+// as a numbered list, which would prevent re-flow

[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 2 inline comments as done.
Szelethus added a comment.

It looks like I won't be back in office until Monday, so I can't finish this 
one until then :(

>> but I haven't seen the current version of the checker crash due to pointer 
>> chasing.
> 
> @NoQ saw quite a few crashes during evaluation, right?

That doesn't sound any good. I'd be very interested in fixing those. Any plans 
to make some bugreports on bugzilla?

>> This might be a little nit-picking, but I don't think the finds are false 
>> positive
> 
> Well, yeah, it depends on how do you define things. If we define reports as 
> "bugs", then I guess this checker already does not find bugs, but 
> best-practices violations.
>  Then it can be argued whether "best practice" should be extended to include 
> all the memory reachable through pointers.

Fair point. I still am very confident that pointer chasing should be a thing, 
but I strongly agree that it lacks good heuristics to make the reports more 
meaningful.


https://reviews.llvm.org/D49438



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

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338832: Fully qualify the renamed symbol if the shortened 
name is ambiguous. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D50189

Files:
  cfe/trunk/lib/Tooling/Core/Lookup.cpp
  cfe/trunk/unittests/Tooling/LookupTest.cpp

Index: cfe/trunk/lib/Tooling/Core/Lookup.cpp
===
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp
@@ -14,6 +14,7 @@
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,6 +115,37 @@
   return false;
 }
 
+// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
+// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
+// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
+// can be resolved to ::a::y::bar, which can cause compile error.
+// FIXME: consider using namespaces.
+static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
+   const DeclContext &UseContext) {
+  assert(QName.startswith("::"));
+  if (Spelling.startswith("::"))
+return false;
+
+  // Lookup the first component of Spelling in all enclosing namespaces and
+  // check if there is any existing symbols with the same name but in different
+  // scope.
+  StringRef Head = Spelling.split("::").first;
+
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(&UseContext);
+  auto &AST = UseContext.getParentASTContext();
+  StringRef TrimmedQName = QName.substr(2);
+  for (const auto *NS : UseNamespaces) {
+auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+if (!LookupRes.empty()) {
+  for (const NamedDecl *Res : LookupRes)
+if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+  return true;
+}
+  }
+  return false;
+}
+
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
const DeclContext *UseContext,
const NamedDecl *FromDecl,
@@ -146,6 +178,14 @@
   // figure out how good a namespace match we have with our destination type.
   // We work backwards (from most specific possible namespace to least
   // specific).
-  return getBestNamespaceSubstr(UseContext, ReplacementString,
-isFullyQualified(Use));
+  StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,
+   isFullyQualified(Use));
+  // Use the fully qualified name if the suggested name is ambiguous.
+  // FIXME: consider re-shortening the name until the name is not ambiguous. We
+  // are not doing this because ambiguity is pretty bad and we should not try to
+  // be clever in handling such cases. Making this noticeable to users seems to
+  // be a better option.
+  return isAmbiguousNameInScope(Suggested, ReplacementString, *UseContext)
+ ? ReplacementString
+ : Suggested;
 }
Index: cfe/trunk/unittests/Tooling/LookupTest.cpp
===
--- cfe/trunk/unittests/Tooling/LookupTest.cpp
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp
@@ -68,7 +68,7 @@
   "namespace b { void f() { a::foo(); } }\n");
 
   Visitor.OnCall = [&](CallExpr *Expr) {
-EXPECT_EQ("a::bar", replaceCallExpr(Expr, "::a::bar"));
+EXPECT_EQ("::a::bar", replaceCallExpr(Expr, "::a::bar"));
   };
   Visitor.runOver("namespace a { void foo(); }\n"
   "namespace b { namespace a { void foo(); }\n"
@@ -127,6 +127,38 @@
   "namespace a { namespace b { namespace c {"
   "void f() { foo(); }"
   "} } }\n");
+
+  // If the shortest name is ambiguous, we need to add more qualifiers.
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("::a::y::bar", replaceCallExpr(Expr, "::a::y::bar"));
+  };
+  Visitor.runOver(R"(
+namespace a {
+namespace b {
+namespace x { void foo() {} }
+namespace y { void foo() {} }
+}
+}
+
+namespace a {
+namespace b {
+void f() { x::foo(); }
+}
+})");
+
+  Visitor.OnCall = [&](CallExpr *Expr) {
+EXPECT_EQ("y::bar", replaceCallExpr(Expr, "::y::bar"));
+  };
+  Visitor.runOver(R"(
+namespace a {
+namespace b {
+namespace x { void foo() {} }
+namespace y { void foo() {} }
+}
+}
+
+void f() { a::b::x::foo(); }
+)");
 }
 
 TEST(LookupTest, replaceNestedClassName) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r338832 - Fully qualify the renamed symbol if the shortened name is ambiguous.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Aug  3 02:16:07 2018
New Revision: 338832

URL: http://llvm.org/viewvc/llvm-project?rev=338832&view=rev
Log:
Fully qualify the renamed symbol if the shortened name is ambiguous.

Summary:
For example, when renaming `a::b::x::foo` to `y::foo` below, replacing
`x::foo()` with `y::foo()` can cause ambiguity. In such cases, we simply fully
qualify the name with leading `::`.
```
namespace a {
namespace b {
namespace x { void foo() {} }
namespace y { void foo() {} }
}
}

namespace a {
namespace b {
void f() { x::foo(); }
}
}
```

Reviewers: ilya-biryukov, hokein

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Tooling/Core/Lookup.cpp
cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=338832&r1=338831&r2=338832&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Fri Aug  3 02:16:07 2018
@@ -14,6 +14,7 @@
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,6 +115,37 @@ static bool isFullyQualified(const Neste
   return false;
 }
 
+// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
+// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
+// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
+// can be resolved to ::a::y::bar, which can cause compile error.
+// FIXME: consider using namespaces.
+static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
+   const DeclContext &UseContext) {
+  assert(QName.startswith("::"));
+  if (Spelling.startswith("::"))
+return false;
+
+  // Lookup the first component of Spelling in all enclosing namespaces and
+  // check if there is any existing symbols with the same name but in different
+  // scope.
+  StringRef Head = Spelling.split("::").first;
+
+  llvm::SmallVector UseNamespaces =
+  getAllNamedNamespaces(&UseContext);
+  auto &AST = UseContext.getParentASTContext();
+  StringRef TrimmedQName = QName.substr(2);
+  for (const auto *NS : UseNamespaces) {
+auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+if (!LookupRes.empty()) {
+  for (const NamedDecl *Res : LookupRes)
+if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+  return true;
+}
+  }
+  return false;
+}
+
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
const DeclContext *UseContext,
const NamedDecl *FromDecl,
@@ -146,6 +178,14 @@ std::string tooling::replaceNestedName(c
   // figure out how good a namespace match we have with our destination type.
   // We work backwards (from most specific possible namespace to least
   // specific).
-  return getBestNamespaceSubstr(UseContext, ReplacementString,
-isFullyQualified(Use));
+  StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,
+   isFullyQualified(Use));
+  // Use the fully qualified name if the suggested name is ambiguous.
+  // FIXME: consider re-shortening the name until the name is not ambiguous. We
+  // are not doing this because ambiguity is pretty bad and we should not try 
to
+  // be clever in handling such cases. Making this noticeable to users seems to
+  // be a better option.
+  return isAmbiguousNameInScope(Suggested, ReplacementString, *UseContext)
+ ? ReplacementString
+ : Suggested;
 }

Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=338832&r1=338831&r2=338832&view=diff
==
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Fri Aug  3 02:16:07 2018
@@ -68,7 +68,7 @@ TEST(LookupTest, replaceNestedFunctionNa
   "namespace b { void f() { a::foo(); } }\n");
 
   Visitor.OnCall = [&](CallExpr *Expr) {
-EXPECT_EQ("a::bar", replaceCallExpr(Expr, "::a::bar"));
+EXPECT_EQ("::a::bar", replaceCallExpr(Expr, "::a::bar"));
   };
   Visitor.runOver("namespace a { void foo(); }\n"
   "namespace b { namespace a { void foo(); }\n"
@@ -127,6 +127,38 @@ TEST(LookupTest, replaceNestedFunctionNa
   "namespace a { namespace b { namespace c {"
   "void f() { foo(); }"
   "} } }\n");
+
+  // If the shortest name is ambiguous,

[PATCH] D50230: clang-format: [JS] don't break comments before any '{'

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Great! Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D50230



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

[PATCH] D50230: clang-format: [JS] don't break comments before any '{'

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338837: clang-format: [JS] don't break comments before 
any '{' (authored by mprobst, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50230?vs=158946&id=158957#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50230

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


Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,19 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.
Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",


Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,19 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOf

r338837 - clang-format: [JS] don't break comments before any '{'

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Aug  3 02:34:41 2018
New Revision: 338837

URL: http://llvm.org/viewvc/llvm-project?rev=338837&view=rev
Log:
clang-format: [JS] don't break comments before any '{'

Summary:
Previously, clang-format would avoid breaking before the first `{`
found, but then happily break before subsequent '{'s on the line. This
change fixes that by looking for the first location that has no opening
curly, if any.

Reviewers: krasimir

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=338837&r1=338836&r2=338837&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Fri Aug  3 02:34:41 2018
@@ -90,19 +90,19 @@ static BreakableToken::Split getCommentS
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=338837&r1=338836&r2=338837&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Aug  3 02:34:41 2018
@@ -2067,6 +2067,14 @@ TEST_F(FormatTestJS, JSDocAnnotations) {
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",


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

[PATCH] D48098: clang-format-diff: Make it work with python3 too

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338839: clang-format-diff: Make it work with python3 too 
(authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48098?vs=152867&id=158961#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48098

Files:
  cfe/trunk/tools/clang-format/clang-format-diff.py


Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -25,10 +25,12 @@
 import argparse
 import difflib
 import re
-import string
 import subprocess
-import StringIO
 import sys
+try:
+  from StringIO import StringIO
+except ImportError:
+   from io import StringIO
 
 
 def main():
@@ -84,36 +86,39 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).extend(
   ['-lines', str(start_line) + ':' + str(end_line)])
 
   # Reformat files containing changes in place.
-  for filename, lines in lines_by_file.iteritems():
+  for filename, lines in lines_by_file.items():
 if args.i and args.verbose:
-  print 'Formatting', filename
+  print('Formatting {}'.format(filename))
 command = [args.binary, filename]
 if args.i:
   command.append('-i')
 if args.sort_includes:
   command.append('-sort-includes')
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command, stdout=subprocess.PIPE,
- stderr=None, stdin=subprocess.PIPE)
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=None,
+ stdin=subprocess.PIPE,
+ universal_newlines=True)
 stdout, stderr = p.communicate()
 if p.returncode != 0:
-  sys.exit(p.returncode);
+  sys.exit(p.returncode)
 
 if not args.i:
   with open(filename) as f:
 code = f.readlines()
-  formatted_code = StringIO.StringIO(stdout).readlines()
+  formatted_code = StringIO(stdout).readlines()
   diff = difflib.unified_diff(code, formatted_code,
   filename, filename,
   '(before formatting)', '(after formatting)')
-  diff_string = string.join(diff, '')
+  diff_string = ''.join(diff)
   if len(diff_string) > 0:
 sys.stdout.write(diff_string)
 


Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -25,10 +25,12 @@
 import argparse
 import difflib
 import re
-import string
 import subprocess
-import StringIO
 import sys
+try:
+  from StringIO import StringIO
+except ImportError:
+   from io import StringIO
 
 
 def main():
@@ -84,36 +86,39 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).extend(
   ['-lines', str(start_line) + ':' + str(end_line)])
 
   # Reformat files containing changes in place.
-  for filename, lines in lines_by_file.iteritems():
+  for filename, lines in lines_by_file.items():
 if args.i and args.verbose:
-  print 'Formatting', filename
+  print('Formatting {}'.format(filename))
 command = [args.binary, filename]
 if args.i:
   command.append('-i')
 if args.sort_includes:
   command.append('-sort-includes')
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command, stdout=subprocess.PIPE,
- stderr=None, stdin=subprocess.PIPE)
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=None,
+ stdin=subprocess.PIPE,
+ universal_newlines=True)
 stdout, stderr = p.communicate()
 if p.returncode != 0:
-  sys.exit(p.returncode);
+  sys.exit(p.returncode)
 
 if not args.i:
   with open(filename) as f:
 code = f.readlines()
-  formatted_code = StringIO.StringIO(stdout).readlines()
+  formatted_code = StringIO(stdout).readlines()
   diff = difflib.unified_diff(code, formatted_code,
   filename, filename,
   '(before formatting)', '(after formatting)')
-  diff_string = string.join(diff, '')
+  diff_string = ''.join(diff)
   if len(diff_st

r338839 - clang-format-diff: Make it work with python3 too

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Fri Aug  3 03:04:58 2018
New Revision: 338839

URL: http://llvm.org/viewvc/llvm-project?rev=338839&view=rev
Log:
clang-format-diff: Make it work with python3 too

Summary: It is not necessary, but would be nice if the script run on python3 as 
well (as opposed to only python2, which is going to be deprecated 
https://pythonclock.org/)

Contributed by MarcoFalke!

Reviewers: krasimir

Reviewed By: krasimir

Subscribers: lebedev.ri, sammccall, cfe-commits

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

Modified:
cfe/trunk/tools/clang-format/clang-format-diff.py

Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=338839&r1=338838&r2=338839&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Fri Aug  3 03:04:58 2018
@@ -25,10 +25,12 @@ Example usage for git/svn users:
 import argparse
 import difflib
 import re
-import string
 import subprocess
-import StringIO
 import sys
+try:
+  from StringIO import StringIO
+except ImportError:
+   from io import StringIO
 
 
 def main():
@@ -84,14 +86,14 @@ def main():
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).extend(
   ['-lines', str(start_line) + ':' + str(end_line)])
 
   # Reformat files containing changes in place.
-  for filename, lines in lines_by_file.iteritems():
+  for filename, lines in lines_by_file.items():
 if args.i and args.verbose:
-  print 'Formatting', filename
+  print('Formatting {}'.format(filename))
 command = [args.binary, filename]
 if args.i:
   command.append('-i')
@@ -100,20 +102,23 @@ def main():
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])
-p = subprocess.Popen(command, stdout=subprocess.PIPE,
- stderr=None, stdin=subprocess.PIPE)
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=None,
+ stdin=subprocess.PIPE,
+ universal_newlines=True)
 stdout, stderr = p.communicate()
 if p.returncode != 0:
-  sys.exit(p.returncode);
+  sys.exit(p.returncode)
 
 if not args.i:
   with open(filename) as f:
 code = f.readlines()
-  formatted_code = StringIO.StringIO(stdout).readlines()
+  formatted_code = StringIO(stdout).readlines()
   diff = difflib.unified_diff(code, formatted_code,
   filename, filename,
   '(before formatting)', '(after formatting)')
-  diff_string = string.join(diff, '')
+  diff_string = ''.join(diff)
   if len(diff_string) > 0:
 sys.stdout.write(diff_string)
 


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

Re: r338749 - Work around more GCC miscompiles exposed by r338464.

[Hans Wennborg via cfe-commits]

Merged in r338845. Thanks!

On Thu, Aug 2, 2018 at 10:48 PM, Richard Smith  wrote:
> (+Hans) +1
>
> On Thu, 2 Aug 2018 at 11:37, Martin Storsjö via cfe-commits
>  wrote:
>>
>> Hans,
>>
>> I think this commit should be merged to the 7.0 release branch; the first
>> half of the GCC workaround made it in before the branch happened, but
>> there was another identical case missing.
>>
>> // Martin
>>
>> On Thu, 2 Aug 2018, Martin Storsjo via cfe-commits wrote:
>>
>> > Author: mstorsjo
>> > Date: Thu Aug  2 11:12:08 2018
>> > New Revision: 338749
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=338749&view=rev
>> > Log:
>> > Work around more GCC miscompiles exposed by r338464.
>> >
>> > This is the same fix as in r338478, for another occurrance of the
>> > same pattern from r338464.
>> >
>> > See gcc.gnu.org/PR86769 for details of the bug.
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r338837 - clang-format: [JS] don't break comments before any '{'

[Tim Northover via cfe-commits]

Hi Martin,

On Fri, 3 Aug 2018 at 10:34, Martin Probst via cfe-commits
 wrote:
> clang-format: [JS] don't break comments before any '{'

This looks like it's triggered a bunch of infinite loops in Clang's
unittests. For example:
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/51705/.

Could you take a look?

Cheers.

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

Re: r338775 - [analyzer] Add a safety check to InnerPointerChecker.

[Alexander Kornienko via cfe-commits]

On Fri, Aug 3, 2018 at 12:20 AM Reka Kovacs via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rkovacs
> Date: Thu Aug  2 15:19:57 2018
> New Revision: 338775
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338775&view=rev
> Log:
> [analyzer] Add a safety check to InnerPointerChecker.
>
> Do not crash if the CXXRecordDecl of an object is not available.
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338775&r1=338774&r2=338775&view=diff
>
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
> (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Thu Aug
> 2 15:19:57 2018
> @@ -129,8 +129,11 @@ bool InnerPointerChecker::isCalledOnStri
>  return false;
>
>QualType ObjTy = ObjRegion->getValueType();
> -  if (ObjTy.isNull() ||
> -  ObjTy->getAsCXXRecordDecl()->getName() != "basic_string")
> +  if (ObjTy.isNull())
> +return false;
> +
> +  CXXRecordDecl *Decl = ObjTy->getAsCXXRecordDecl();
> +  if (!Decl || Decl->getName() != "basic_string")
>  return false;
>
>return true;
>

Thanks for the fix! An improvement suggestion:
return Decl && Decl->getName() == "basic_string";


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

r338831 - Fix unused variable warning in tablegen generated code

[Karl-Johan Karlsson via cfe-commits]

Author: karka
Date: Fri Aug  3 02:13:15 2018
New Revision: 338831

URL: http://llvm.org/viewvc/llvm-project?rev=338831&view=rev
Log:
Fix unused variable warning in tablegen generated code

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

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=338831&r1=338830&r2=338831&view=diff
==
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Fri Aug  3 02:13:15 2018
@@ -3274,11 +3274,15 @@ static std::string GenerateCustomApperta
 return "";
   }
 
+  const StringRef CheckCodeValue = Subject.getValueAsString("CheckCode");
+
   OS << "static bool " << FnName << "(const Decl *D) {\n";
-  OS << "  if (const auto *S = dyn_cast<";
-  OS << GetSubjectWithSuffix(Base);
-  OS << ">(D))\n";
-  OS << "return " << Subject.getValueAsString("CheckCode") << ";\n";
+  if (CheckCodeValue != "false") {
+OS << "  if (const auto *S = dyn_cast<";
+OS << GetSubjectWithSuffix(Base);
+OS << ">(D))\n";
+OS << "return " << Subject.getValueAsString("CheckCode") << ";\n";
+  }
   OS << "  return false;\n";
   OS << "}\n\n";
 


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

[libcxxabi] r338860 - Creating release directory for release_700.

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:33:00 2018
New Revision: 338860

URL: http://llvm.org/viewvc/llvm-project?rev=338860&view=rev
Log:
Creating release directory for release_700.

Added:
libcxxabi/tags/RELEASE_700/

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

[libcxx] r338858 - Creating release directory for release_700.

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:32:47 2018
New Revision: 338858

URL: http://llvm.org/viewvc/llvm-project?rev=338858&view=rev
Log:
Creating release directory for release_700.

Added:
libcxx/tags/RELEASE_700/

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

[libcxx] r338859 - Creating release candidate rc1 from release_700 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:32:56 2018
New Revision: 338859

URL: http://llvm.org/viewvc/llvm-project?rev=338859&view=rev
Log:
Creating release candidate rc1 from release_700 branch

Added:
libcxx/tags/RELEASE_700/rc1/   (props changed)
  - copied from r338858, libcxx/branches/release_70/

Propchange: libcxx/tags/RELEASE_700/rc1/
--
svn:mergeinfo = /libcxx/branches/apple:136569-137939


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

[libcxxabi] r338861 - Creating release candidate rc1 from release_700 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:33:05 2018
New Revision: 338861

URL: http://llvm.org/viewvc/llvm-project?rev=338861&view=rev
Log:
Creating release candidate rc1 from release_700 branch

Added:
libcxxabi/tags/RELEASE_700/rc1/
  - copied from r338860, libcxxabi/branches/release_70/

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

[libunwind] r338873 - Creating release candidate rc1 from release_700 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:34:07 2018
New Revision: 338873

URL: http://llvm.org/viewvc/llvm-project?rev=338873&view=rev
Log:
Creating release candidate rc1 from release_700 branch

Added:
libunwind/tags/RELEASE_700/rc1/
  - copied from r338872, libunwind/branches/release_70/

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

[libunwind] r338872 - Creating release directory for release_700.

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug  3 04:34:02 2018
New Revision: 338872

URL: http://llvm.org/viewvc/llvm-project?rev=338872&view=rev
Log:
Creating release directory for release_700.

Added:
libunwind/tags/RELEASE_700/

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

[PATCH] D50110: Handle shared release attributes correctly

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085
+  void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+  }
+
+  void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {

aaron.ballman wrote:
> Nothing calls either of these functions within the test; is that intended?
Yes, I just wanted to check that there are no warnings within the functions. 
Before the changes in `lib/Analysis/ThreadSafety.cpp`, we would get a warning 
"releasing mutex 'mu' using shared access, expected exclusive access" on line 
4086.

My changes address the attributes on the function being analyzed, not on a 
function that is called from the function being analyzed.


Repository:
  rC Clang

https://reviews.llvm.org/D50110



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

[PATCH] D49873: [Docs] ReleasesNotes update / Static analyser

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping :)


Repository:
  rC Clang

https://reviews.llvm.org/D49873



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

[PATCH] D49906: [Docs] Sanitizer update

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping :)


Repository:
  rC Clang

https://reviews.llvm.org/D49906



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

Re: r338837 - clang-format: [JS] don't break comments before any '{'

[Tim Northover via cfe-commits]

On Fri, 3 Aug 2018 at 11:47, Tim Northover  wrote:
> On Fri, 3 Aug 2018 at 10:34, Martin Probst via cfe-commits
>  wrote:
> > clang-format: [JS] don't break comments before any '{'
>
> This looks like it's triggered a bunch of infinite loops in Clang's
> unittests.

I've just taken a look at the normal lab.llvm.org bots and it looks
like *everything* is broken so I've reverted it for now.

Cheers.

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

r338879 - Revert "clang-format: [JS] don't break comments before any '{'"

[Tim Northover via cfe-commits]

Author: tnorthover
Date: Fri Aug  3 05:19:22 2018
New Revision: 338879

URL: http://llvm.org/viewvc/llvm-project?rev=338879&view=rev
Log:
Revert "clang-format: [JS] don't break comments before any '{'"

This reverts commit r338837, it introduced an infinite loop on all bots.

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=338879&r1=338878&r2=338879&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Fri Aug  3 05:19:22 2018
@@ -90,19 +90,19 @@ static BreakableToken::Split getCommentS
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
+  // Do not split before a number followed by a dot: this would be interpreted
+  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  while (SpaceOffset != StringRef::npos) {
-// Do not split before a number followed by a dot: this would be 
interpreted
-// as a numbered list, which would prevent re-flowing in subsequent passes.
-if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-// In JavaScript, some @tags can be followed by {, and machinery that 
parses
-// these comments will fail to understand the comment if followed by a line
-// break. So avoid ever breaking before a {.
-else if (Style.Language == FormatStyle::LK_JavaScript &&
- SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
-  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  }
+  if (SpaceOffset != StringRef::npos &&
+  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  // In JavaScript, some @tags can be followed by {, and machinery that parses
+  // these comments will fail to understand the comment if followed by a line
+  // break. So avoid ever breaking before a {.
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
+  Text[SpaceOffset + 1] == '{')
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=338879&r1=338878&r2=338879&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Aug  3 05:19:22 2018
@@ -2067,14 +2067,6 @@ TEST_F(FormatTestJS, JSDocAnnotations) {
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
-  // make sure clang-format doesn't break before *any* '{'
-  verifyFormat("/**\n"
-   " * @lala {lala {lalala\n"
-   " */\n",
-   "/**\n"
-   " * @lala {lala {lalala\n"
-   " */\n",
-   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",


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

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

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill created this revision.
lewis-revill added reviewers: asb, simoncook, edward-jones.
Herald added subscribers: cfe-commits, rkruppe, the_o, brucehoult, 
MartinMosbeck, rogfer01, mgrang, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, apazos, johnrusso, rbar.

Extends r338385 to allow the driver to compute the sysroot when an explicit 
path is not provided. This allows the linker to find C runtime files and the 
correct include directory for header files.


Repository:
  rC Clang

https://reviews.llvm.org/D50246

Files:
  lib/Driver/ToolChains/RISCV.cpp
  lib/Driver/ToolChains/RISCV.h


Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolChains/RISCV.h
+++ lib/Driver/ToolChains/RISCV.h
@@ -32,6 +32,9 @@
 
 protected:
   Tool *buildLinker() const override;
+
+private:
+  std::string computeSysRoot() const;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/RISCV.cpp
===
--- lib/Driver/ToolChains/RISCV.cpp
+++ lib/Driver/ToolChains/RISCV.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -27,7 +28,7 @@
const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
-  getFilePaths().push_back(D.SysRoot + "/lib");
+  getFilePaths().push_back(computeSysRoot() + "/lib");
   if (GCCInstallation.isValid()) {
 getFilePaths().push_back(GCCInstallation.getInstallPath().str());
 getProgramPaths().push_back(
@@ -45,24 +46,39 @@
 return;
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");
 addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 }
 
 void RISCVToolChain::addLibStdCxxIncludePaths(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
-  StringRef LibDir = GCCInstallation.getParentLibPath();
   const GCCVersion &Version = GCCInstallation.getVersion();
   StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
-  addLibStdCXXIncludePaths(
-  LibDir.str() + "/../" + TripleStr.str() + "/include/c++/" + Version.Text,
+  addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
   "", TripleStr, "", "", Multilib.includeSuffix(), DriverArgs, CC1Args);
 }
 
+std::string RISCVToolChain::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+
+  if (!GCCInstallation.isValid())
+return std::string();
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+  std::string SysRootDir = LibDir.str() + "/../" + TripleStr.str();
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+return std::string();
+
+  return SysRootDir;
+}
+
 void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
  const InputInfo &Output,
  const InputInfoList &Inputs,


Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolChains/RISCV.h
+++ lib/Driver/ToolChains/RISCV.h
@@ -32,6 +32,9 @@
 
 protected:
   Tool *buildLinker() const override;
+
+private:
+  std::string computeSysRoot() const;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/RISCV.cpp
===
--- lib/Driver/ToolChains/RISCV.cpp
+++ lib/Driver/ToolChains/RISCV.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -27,7 +28,7 @@
const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
-  getFilePaths().push_back(D.SysRoot + "/lib");
+  getFilePaths().push_back(computeSysRoot() + "/lib");
   if (GCCInstallation.isValid()) {
 getFilePaths().push_back(GCCInstallation.getInstallPath().str());
 getProgramPaths().push_back(
@@ -45,24 +46,39 @@
 return;
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");
 addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 }
 
 void RISCVToolChain::addLibStdCxxIncludePaths(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
-  StringRef LibDir = GCCInstallation.getParentLib

[PATCH] D46822: [RISCV] Add driver for riscv32-unknown-elf baremetal target

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill added a comment.

I've submitted a patch to address Simon's issues in 
https://reviews.llvm.org/D50246


Repository:
  rC Clang

https://reviews.llvm.org/D46822



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

Re: r338259 - [analyzer] Add support for more invalidating functions in InnerPointerChecker.

[Alexander Kornienko via cfe-commits]

Thanks for the prompt fix, it solved the problem.

In case you want to add a regression test, this is what I came up with:
$ cat test-isCalledOnStringObject.cc
char *c();
class B {};
void d() {
  B g, *h;
  *(void **)&h = c() + 1;
  *h = g;
}
$ ./clang-tidy -checks=-*,clang-analyzer* test-isCalledOnStringObject.cc --
-std=c++11
@ 0x5640f3e56b85 32  clang::DeclarationName::isIdentifier()
@ 0x5640f3ebaca3 80  clang::NamedDecl::getName()
@ 0x5640f9f7e56b336  (anonymous
namespace)::InnerPointerChecker::isCalledOnStringObject()
@ 0x5640f9f7e0ef288  (anonymous
namespace)::InnerPointerChecker::checkPostCall()
@ 0x5640f9f7e080 48
clang::ento::check::PostCall::_checkCall<>()
@ 0x5640fa2d5c12 64  clang::ento::CheckerFn<>::operator()()
@ 0x5640fa2c82d8208  (anonymous
namespace)::CheckCallContext::runChecker()
@ 0x5640fa2c4d6c384  expandGraphWithCheckers<>()
@ 0x5640fa2c4acb208
clang::ento::CheckerManager::runCheckersForCallEvent()
@ 0x5640fa32fdc8 80
clang::ento::CheckerManager::runCheckersForPostCall()
@ 0x5640fa332ff3336  clang::ento::ExprEngine::evalCall()
@ 0x5640fa332e89400
clang::ento::ExprEngine::VisitCallExpr()
@ 0x5640fa2ef8cb   4304  clang::ento::ExprEngine::Visit()
@ 0x5640fa2ec38c464  clang::ento::ExprEngine::ProcessStmt()
@ 0x5640fa2ec079208
clang::ento::ExprEngine::processCFGElement()
@ 0x5640fa31d8ff128
clang::ento::CoreEngine::HandlePostStmt()
@ 0x5640fa31d208496
clang::ento::CoreEngine::dispatchWorkItem()
@ 0x5640fa31cdbb544
clang::ento::CoreEngine::ExecuteWorkList()
@ 0x5640f9c466b4 80
clang::ento::ExprEngine::ExecuteWorkList()


On Fri, Aug 3, 2018 at 12:28 AM Réka Nikolett Kovács 
wrote:

> Thanks for the notice. Have you managed to get the repro? I haven't
> succeeded in constructing one yet, but I've committed a check for the
> CXXRecordDecl. I hope that fixes the crashes.
>
> Alexander Kornienko  ezt írta (időpont: 2018. aug. 2.,
> Cs, 11:07):
>
>>
>>
>> On Mon, Jul 30, 2018 at 5:44 PM Reka Kovacs via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: rkovacs
>>> Date: Mon Jul 30 08:43:45 2018
>>> New Revision: 338259
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=338259&view=rev
>>> Log:
>>> [analyzer] Add support for more invalidating functions in
>>> InnerPointerChecker.
>>>
>>> According to the standard, pointers referring to the elements of a
>>> `basic_string` may be invalidated if they are used as an argument to
>>> any standard library function taking a reference to non-const
>>> `basic_string` as an argument. This patch makes InnerPointerChecker warn
>>> for these cases.
>>>
>>> Differential Revision: https://reviews.llvm.org/D49656
>>>
>>> Modified:
>>> cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
>>> cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
>>> cfe/trunk/test/Analysis/inner-pointer.cpp
>>>
>>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338259&r1=338258&r2=338259&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
>>> (original)
>>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Mon
>>> Jul 30 08:43:45 2018
>>> @@ -91,37 +91,53 @@ public:
>>>  ReserveFn("reserve"), ResizeFn("resize"),
>>>  ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
>>>
>>> -  /// Check whether the function called on the container object is a
>>> -  /// member function that potentially invalidates pointers referring
>>> -  /// to the objects's internal buffer.
>>> -  bool mayInvalidateBuffer(const CallEvent &Call) const;
>>> -
>>> -  /// Record the connection between the symbol returned by c_str() and
>>> the
>>> -  /// corresponding string object region in the ProgramState. Mark the
>>> symbol
>>> -  /// released if the string object is destroyed.
>>> +  /// Check if the object of this member function call is a
>>> `basic_string`.
>>> +  bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
>>> +
>>> +  /// Check whether the called member function potentially invalidates
>>> +  /// pointers referring to the container object's inner buffer.
>>> +  bool isInvalidatingMemberFunction(const CallEvent &Call) const;
>>> +
>>> +  /// Mark pointer symbols associated with the given memory region
>>> released
>>> +  /// in the program state.
>>> +  void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef
>>> State,
>>> +  const MemRegion *ObjRegion,
>>> +  CheckerContext &C) const;
>>> +
>>> +  /// 

[PATCH] D48100: Append new attributes to the end of an AttributeList.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:

> I have two approaches to tackle the wrong marker order: 
> https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO 
> both are too invasive to be justified for the small issue.


I think you're right here.  I despise https://reviews.llvm.org/D50215, and only 
mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer leaving this 
as a "FIXME" somewhere.


Repository:
  rL LLVM

https://reviews.llvm.org/D48100



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

[PATCH] D50214: Add inherited attributes before parsed attributes.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

IMO, I think this (and the next 2 numerically), show to me that we have too 
much dependence on the order of attributes, which all of these show is not a 
reliable thing to count on.  I quite dislike this as well, and think this is 
going to lead us to chasing these same issues forever.

I wonder if the solution here is to just sort the attributes by source 
location?  This would create that guarantee, which previously didn't exist.


Repository:
  rC Clang

https://reviews.llvm.org/D50214



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

Re: r338837 - clang-format: [JS] don't break comments before any '{'

[Martin Probst via cfe-commits]

Sorry about that, yes, that was me. Thanks for reverting!

Tim Northover  schrieb am Fr., 3. Aug. 2018 um
14:19 Uhr:

> On Fri, 3 Aug 2018 at 11:47, Tim Northover 
> wrote:
> > On Fri, 3 Aug 2018 at 10:34, Martin Probst via cfe-commits
> >  wrote:
> > > clang-format: [JS] don't break comments before any '{'
> >
> > This looks like it's triggered a bunch of infinite loops in Clang's
> > unittests.
>
> I've just taken a look at the normal lab.llvm.org bots and it looks
> like *everything* is broken so I've reverted it for now.
>
> Cheers.
>
> Tim.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Aug  3 06:01:32 2018
New Revision: 338884

URL: http://llvm.org/viewvc/llvm-project?rev=338884&view=rev
Log:
[NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338884&r1=338883&r2=338884&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:01:32 2018
@@ -145,7 +145,7 @@ def HasFunctionProto : SubsetSubject;
 
 // A single argument to an attribute


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

[PATCH] D50249: clang-format: [JS] don't break comments before any '{'

[Martin Probst via Phabricator via cfe-commits]

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

Previously, clang-format would avoid breaking before the first `{`
found, but then happily break before subsequent '{'s on the line. This
change fixes that by looking for the first location that has no opening
curly, if any.

This fixes the original commit by correcting the loop condition.

This reverts commit 66dc646e09b795b943668179c33d09da71a3b6bc.


Repository:
  rC Clang

https://reviews.llvm.org/D50249

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


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,21 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+else
+  break;
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,21 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyl

Re: r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

[Nico Weber via cfe-commits]

This causes a warning:

gen/clang/include/clang/Sema/AttrParsedAttrImpl.inc:1050:12: warning:
expression result unused [-Wunused-value]
return S, false;
   ^

On Fri, Aug 3, 2018 at 9:01 AM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Fri Aug  3 06:01:32 2018
> New Revision: 338884
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338884&view=rev
> Log:
> [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338884&r1=338883&r2=338884&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:01:32 2018
> @@ -145,7 +145,7 @@ def HasFunctionProto : SubsetSubject  // function. Accepted as a function type attribute on the type of such a
>  // member function.
>  // FIXME: This does not actually ever match currently.
> -def ImplicitObjectParameter : SubsetSubject +def ImplicitObjectParameter : SubsetSubject  "implicit object parameters">;
>
>  // A single argument to an attribute
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r338385 - [RISCV] Add driver for riscv32-unknown-elf baremetal target

[Nico Weber via cfe-commits]

I'm getting this warning from the mac linker after this commit:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
warning same member name (libclangDriver.RISCV.o) in output file used for
input files: obj/clang/lib/Driver/ToolChains/Arch/libclangDriver.RISCV.o
and: obj/clang/lib/Driver/ToolChains/libclangDriver.RISCV.o (due to use of
basename, truncation, blank padding or duplicate input files)

Could we rename the file to fix that warning?

On Tue, Jul 31, 2018 at 10:40 AM David Bolvansky via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: xbolva00
> Date: Tue Jul 31 07:21:46 2018
> New Revision: 338385
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338385&view=rev
> Log:
> [RISCV] Add driver for riscv32-unknown-elf baremetal target
>
> Summary:
> This patch adds a driver for the baremetal RISC-V target (i.e.
> riscv32-unknown-elf). For reference, D39963 added basic target info and
> added support for riscv32-linux-unknown-elf.
>
> Patch by: asb (Alex Bradbury)
>
> Reviewers: efriedma, phosek, apazos, espindola, mgrang
>
> Reviewed By: mgrang
>
> Subscribers: jrtc27, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe,
> emaste, mgorny, arichardson, rbar, johnrusso, simoncook,
> jordy.potman.lists, sabuasal, niosHD, kito-cheng, shiva0217, zzheng,
> edward-jones, mgrang, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D46822
>
> Added:
> cfe/trunk/lib/Driver/ToolChains/RISCV.cpp
> cfe/trunk/lib/Driver/ToolChains/RISCV.h
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/riscv32-unknown-elf-ld
>  (with props)
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtbegin.o
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtend.o
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/.keep
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/
>
> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/crt0.o
> Modified:
> cfe/trunk/lib/Driver/CMakeLists.txt
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> cfe/trunk/test/Driver/riscv32-toolchain.c
>
> Modified: cfe/trunk/lib/Driver/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=338385&r1=338384&r2=338385&view=diff
>
> ==
> --- cfe/trunk/lib/Driver/CMakeLists.txt (original)
> +++ cfe/trunk/lib/Driver/CMakeLists.txt Tue Jul 31 07:21:46 2018
> @@ -57,6 +57,7 @@ add_clang_library(clangDriver
>ToolChains/NetBSD.cpp
>ToolChains/OpenBSD.cpp
>ToolChains/PS4CPU.cpp
> +  ToolChains/RISCV.cpp
>ToolChains/Solaris.cpp
>ToolChains/TCE.cpp
>ToolChains/WebAssembly.cpp
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=338385&r1=338384&r2=338385&view=diff
>
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Jul 31 07:21:46 2018
> @@ -37,6 +37,7 @@
>  #include "ToolChains/NetBSD.h"
>  #include "ToolChains/OpenBSD.h"
>  #include "ToolChains/PS4CPU.h"
> +#include "ToolChains/RISCV.h"
>  #include "ToolChains/Solaris.h"
>  #include "ToolChains/TCE.h"
>  #include "ToolChains/WebAssembly.h"
> @@ -4399,6 +4400,10 @@ const ToolChain &Driver::getToolChain(co
>case llvm::Triple::avr:
>  TC = llvm::make_unique(*this, Target,
> Args);
>  break;
> +  case llvm::Triple::riscv32:
> +  case llvm::Triple::riscv64:
> +TC = llvm::make_unique(*this, Target,
> Args);
> +break;
>default:
>  if (Target.getVendor() == llvm::Triple::Myriad)
>TC = llvm::make_unique(*this,
> Target,
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=338385&r1=338384&r2=338385&view=diff
>
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (ori

RE: r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

[Keane, Erich via cfe-commits]

Hmm… I thought this was a pretty clever one.  Again, that didn’t show up in my 
older GCC :/

Any thoughts on how I can silence this AND the other warning?



From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, August 3, 2018 6:16 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r338884 - [NFC] Silence unused variable warning in 
Attr.td/AttrParsedAttrImpl.inc

This causes a warning:

gen/clang/include/clang/Sema/AttrParsedAttrImpl.inc:1050:12: warning: 
expression result unused [-Wunused-value]
return S, false;
   ^

On Fri, Aug 3, 2018 at 9:01 AM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Aug  3 06:01:32 2018
New Revision: 338884

URL: http://llvm.org/viewvc/llvm-project?rev=338884&view=rev
Log:
[NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338884&r1=338883&r2=338884&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:01:32 2018
@@ -145,7 +145,7 @@ def HasFunctionProto : SubsetSubject;

 // A single argument to an attribute


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

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

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 158994.
lewis-revill added a comment.

Modified test which assumed the path to C++ includes was determined without use 
of the sysroot path.


Repository:
  rC Clang

https://reviews.llvm.org/D50246

Files:
  lib/Driver/ToolChains/RISCV.cpp
  lib/Driver/ToolChains/RISCV.h
  test/Driver/riscv32-toolchain.c


Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -24,7 +24,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
-// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: 
"--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolChains/RISCV.h
+++ lib/Driver/ToolChains/RISCV.h
@@ -32,6 +32,9 @@
 
 protected:
   Tool *buildLinker() const override;
+
+private:
+  std::string computeSysRoot() const;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/RISCV.cpp
===
--- lib/Driver/ToolChains/RISCV.cpp
+++ lib/Driver/ToolChains/RISCV.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -27,7 +28,7 @@
const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
-  getFilePaths().push_back(D.SysRoot + "/lib");
+  getFilePaths().push_back(computeSysRoot() + "/lib");
   if (GCCInstallation.isValid()) {
 getFilePaths().push_back(GCCInstallation.getInstallPath().str());
 getProgramPaths().push_back(
@@ -45,24 +46,39 @@
 return;
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");
 addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 }
 
 void RISCVToolChain::addLibStdCxxIncludePaths(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
-  StringRef LibDir = GCCInstallation.getParentLibPath();
   const GCCVersion &Version = GCCInstallation.getVersion();
   StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
-  addLibStdCXXIncludePaths(
-  LibDir.str() + "/../" + TripleStr.str() + "/include/c++/" + Version.Text,
+  addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
   "", TripleStr, "", "", Multilib.includeSuffix(), DriverArgs, CC1Args);
 }
 
+std::string RISCVToolChain::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+
+  if (!GCCInstallation.isValid())
+return std::string();
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+  std::string SysRootDir = LibDir.str() + "/../" + TripleStr.str();
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+return std::string();
+
+  return SysRootDir;
+}
+
 void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
  const InputInfo &Output,
  const InputInfoList &Inputs,


Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -24,7 +24,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
-// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: "--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-el

r338886 - [NFC] Fix unused expression warning introduced in r338884

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Aug  3 06:23:04 2018
New Revision: 338886

URL: http://llvm.org/viewvc/llvm-project?rev=338886&view=rev
Log:
[NFC] Fix unused expression warning introduced in r338884

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338886&r1=338885&r2=338886&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:23:04 2018
@@ -145,8 +145,9 @@ def HasFunctionProto : SubsetSubject;
+def ImplicitObjectParameter
+: SubsetSubject(S), false} ],
+"implicit object parameters">;
 
 // A single argument to an attribute
 class Argument {


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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: vsk, rsmith, rjmccall, Sanitizers.
lebedev.ri added a project: Sanitizers.

C and C++ are interesting languages. They are statically typed, but weakly.
The implicit conversions are allowed. This is nice, allows to write code
while balancing between getting drowned in everything being convertible,
and nothing being convertible. As usual, this comes with a price:

  void consume(unsigned int val);
  
  void test(int val) {
consume(val);
// The 'val' is `signed int`, but `consume()` takes `unsigned int`.
// If val is negative, then consume() will be operating on a large
// unsigned value, and you may or may not have a bug.
  
// But yes, sometimes this is intentional.
// Making the conversion explicit silences the sanitizer.
consume((unsigned int)val);
  }

Yes, there is a `-Wsign-conversion`` diagnostic group, but first, it is kinda
noisy, since it warns on everything (unlike sanitizers, warning on an
actual issues), and second, likely there are cases where it does **not** warn.

The actual detection is pretty easy. We just need to check each of the values
whether it is negative, and equality-compare the results of those comparisons.
The unsigned value is obviously non-negative. Zero is non-negative too.
https://godbolt.org/g/w93oj2

We do not have to emit the check *always*, there are obvious situations
where we can avoid emitting it, since it would **always** get optimized-out.
But i do think the tautological IR (`icmp ult %x, 0`, which is always false)
should be emitted, and the middle-end should cleanup it.

This sanitizer is in the `-fsanitize=implicit-conversion` group,
and is a logical continuation of https://reviews.llvm.org/D48958 
`-fsanitize=implicit-integer-truncation`.
As for the ordering, i'we opted to emit the check **after**
`-fsanitize=implicit-integer-truncation`. At least on these simple 16 test 
cases,
this results in 1 of the 12 emitted checks being optimized away,
as compared to 0 checks being optimized away if the order is reversed.

This is a clang part.
The compiler-rt part is ???.

Finishes fixing PR21530 , PR37552 
, PR35409 
.
Finishes partially fixing PR9821 .
Finishes fixing https://github.com/google/sanitizers/issues/940.

Only the bitfield handling is missing.


Repository:
  rC Clang

https://reviews.llvm.org/D50250

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/catch-implicit-integer-conversions.c
  test/CodeGen/catch-implicit-integer-sign-changes-basics.c
  test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  test/CodeGen/catch-implicit-integer-sign-changes.c
  test/CodeGen/catch-implicit-integer-truncations.c
  test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,22 +29,22 @@
 // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation|implicit-integer-sign-change),?){7}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
-// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1

RE: r338884 - [NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

[Keane, Erich via cfe-commits]

Thanks for the heads up!  I repro’ed both in Godbolt, and saw that clang 
suggests wrapping in a void-cast, so I’ve done  that.

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, August 3, 2018 6:16 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r338884 - [NFC] Silence unused variable warning in 
Attr.td/AttrParsedAttrImpl.inc

This causes a warning:

gen/clang/include/clang/Sema/AttrParsedAttrImpl.inc:1050:12: warning: 
expression result unused [-Wunused-value]
return S, false;
   ^

On Fri, Aug 3, 2018 at 9:01 AM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Aug  3 06:01:32 2018
New Revision: 338884

URL: http://llvm.org/viewvc/llvm-project?rev=338884&view=rev
Log:
[NFC] Silence unused variable warning in Attr.td/AttrParsedAttrImpl.inc

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338884&r1=338883&r2=338884&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:01:32 2018
@@ -145,7 +145,7 @@ def HasFunctionProto : SubsetSubject;

 // A single argument to an attribute


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

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

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Nico Webber (cfe-commits)

"I'm getting this warning from the mac linker after this commit:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
 warning same member name (libclangDriver.RISCV.o) in output file used for 
input files: obj/clang/lib/Driver/ToolChains/Arch/libclangDriver.RISCV.o and: 
obj/clang/lib/Driver/ToolChains/libclangDriver.RISCV.o (due to use of basename, 
truncation, blank padding or duplicate input files)

Could we rename the file to fix that warning?"


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D49790: [AST] Small doc update for DeclContext

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338887: [AST][NFC] Small doc update for DeclContext 
(authored by brunoricci, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D49790

Files:
  include/clang/AST/DeclBase.h


Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1250,16 +1250,29 @@
 /// that directly derive from DeclContext are mentioned, not their subclasses):
 ///
 ///   TranslationUnitDecl
+///   ExternCContext
 ///   NamespaceDecl
-///   FunctionDecl
 ///   TagDecl
+///   OMPDeclareReductionDecl
+///   FunctionDecl
 ///   ObjCMethodDecl
 ///   ObjCContainerDecl
 ///   LinkageSpecDecl
 ///   ExportDecl
 ///   BlockDecl
-///   OMPDeclareReductionDecl
+///   CapturedDecl
 class DeclContext {
+  /// For makeDeclVisibleInContextImpl
+  friend class ASTDeclReader;
+  /// For reconcileExternalVisibleStorage, CreateStoredDeclsMap,
+  /// hasNeedToReconcileExternalVisibleStorage
+  friend class ExternalASTSource;
+  /// For CreateStoredDeclsMap
+  friend class DependentDiagnostic;
+  /// For hasNeedToReconcileExternalVisibleStorage,
+  /// hasLazyLocalLexicalLookups, hasLazyExternalLexicalLookups
+  friend class ASTWriter;
+
   // We use uint64_t in the bit-fields below since some bit-fields
   // cross the unsigned boundary and this breaks the packing.
 
@@ -1716,10 +1729,6 @@
   "BlockDeclBitfields is larger than 8 bytes!");
   };
 
-  friend class ASTDeclReader;
-  friend class ASTWriter;
-  friend class ExternalASTSource;
-
   /// FirstDecl - The first declaration stored within this declaration
   /// context.
   mutable Decl *FirstDecl = nullptr;
@@ -2398,8 +2407,6 @@
 DeclContextBits.HasLazyExternalLexicalLookups = HasLELL;
   }
 
-  friend class DependentDiagnostic;
-
   void reconcileExternalVisibleStorage() const;
   bool LoadLexicalDeclsFromExternalStorage() const;
 


Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1250,16 +1250,29 @@
 /// that directly derive from DeclContext are mentioned, not their subclasses):
 ///
 ///   TranslationUnitDecl
+///   ExternCContext
 ///   NamespaceDecl
-///   FunctionDecl
 ///   TagDecl
+///   OMPDeclareReductionDecl
+///   FunctionDecl
 ///   ObjCMethodDecl
 ///   ObjCContainerDecl
 ///   LinkageSpecDecl
 ///   ExportDecl
 ///   BlockDecl
-///   OMPDeclareReductionDecl
+///   CapturedDecl
 class DeclContext {
+  /// For makeDeclVisibleInContextImpl
+  friend class ASTDeclReader;
+  /// For reconcileExternalVisibleStorage, CreateStoredDeclsMap,
+  /// hasNeedToReconcileExternalVisibleStorage
+  friend class ExternalASTSource;
+  /// For CreateStoredDeclsMap
+  friend class DependentDiagnostic;
+  /// For hasNeedToReconcileExternalVisibleStorage,
+  /// hasLazyLocalLexicalLookups, hasLazyExternalLexicalLookups
+  friend class ASTWriter;
+
   // We use uint64_t in the bit-fields below since some bit-fields
   // cross the unsigned boundary and this breaks the packing.
 
@@ -1716,10 +1729,6 @@
   "BlockDeclBitfields is larger than 8 bytes!");
   };
 
-  friend class ASTDeclReader;
-  friend class ASTWriter;
-  friend class ExternalASTSource;
-
   /// FirstDecl - The first declaration stored within this declaration
   /// context.
   mutable Decl *FirstDecl = nullptr;
@@ -2398,8 +2407,6 @@
 DeclContextBits.HasLazyExternalLexicalLookups = HasLELL;
   }
 
-  friend class DependentDiagnostic;
-
   void reconcileExternalVisibleStorage() const;
   bool LoadLexicalDeclsFromExternalStorage() const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 158997.
theraven added a comment.

- Address Dustin's review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/EHScopeStack.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

Any comments?

https://reviews.llvm.org/D49911, on which this relies, is now ready to land.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49910



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+  // We ignore conversions to/from pointer and/or bool.
+  if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+return;

I'd rather !SrcType->isInt || !DestType->isInt



Comment at: lib/CodeGen/CGExprScalar.cpp:1011
+
+  assert(isa(SrcTy) && isa(DstTy) &&
+ "clang integer type lowered to non-integer llvm type");

This seems like a silly assert, since you did the check above.



Comment at: lib/CodeGen/CGExprScalar.cpp:1030
+  {
+// At least one of the values needs to have signed type.
+// If both are unsigned, then obviously, neither of them can be negative.

Does this really need its own scope?



Comment at: lib/CodeGen/CGExprScalar.cpp:1032
+// If both are unsigned, then obviously, neither of them can be negative.
+if (!(SrcSigned || DstSigned))
+  return;

Again, I'd rather we distribute the '!'.



Comment at: lib/CodeGen/CGExprScalar.cpp:1035
+  }
+  {
+// If the conversion is to *larger* *signed* type, then no check is needed.

These scopes are getting out of hand... just kill them all.  Introducing 
CanonSrcType/CanonDstType into the larger scope isn't that big of a deal.



Comment at: lib/CodeGen/CGExprScalar.cpp:1044
+
+  assert(!DstType->isBooleanType() && "we should not get here with booleans.");
+

Curious what prevents this?



Comment at: lib/CodeGen/CGExprScalar.cpp:1054
+   const char *Name) -> Value * {
+// Does this Value has signed type?
+bool VSigned = VType->isSignedIntegerOrEnumerationType();


// Is this value a signed type?



Comment at: lib/CodeGen/CGExprScalar.cpp:2004
+if (auto *ICE = dyn_cast(CE)) {
+  if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) &&
+  !ICE->isPartOfExplicitCast()) {

Is this an error?  You swapped a 'has' with a 'hasOneOf' but only listed a 
single thing.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:2004
+if (auto *ICE = dyn_cast(CE)) {
+  if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) &&
+  !ICE->isPartOfExplicitCast()) {

erichkeane wrote:
> Is this an error?  You swapped a 'has' with a 'hasOneOf' but only listed a 
> single thing.
`ImplicitConversion` is a *group*, which includes `ImplicitIntegerTruncation` 
and `ImplicitIntegerSignChange`.
So `hasOneOf(group)` checks that at least one check of the group is enabled.
No, this is correct, else the tests would break.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:2004
+if (auto *ICE = dyn_cast(CE)) {
+  if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) &&
+  !ICE->isPartOfExplicitCast()) {

lebedev.ri wrote:
> erichkeane wrote:
> > Is this an error?  You swapped a 'has' with a 'hasOneOf' but only listed a 
> > single thing.
> `ImplicitConversion` is a *group*, which includes `ImplicitIntegerTruncation` 
> and `ImplicitIntegerSignChange`.
> So `hasOneOf(group)` checks that at least one check of the group is enabled.
> No, this is correct, else the tests would break.
Ah! I missed that subtly, just LOOKed odd.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

r338889 - [NFCI] My attempt to fix a warning in r338886 broke the build! Fix it.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Aug  3 06:51:35 2018
New Revision: 338889

URL: http://llvm.org/viewvc/llvm-project?rev=338889&view=rev
Log:
[NFCI] My attempt to fix a warning in r338886 broke the build! Fix it.

Clang format got the best of me... it introduced spaces around something 
in a table-genned file, so it was interpreted as an array and not a 
code block.  

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=338889&r1=33&r2=338889&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Aug  3 06:51:35 2018
@@ -146,7 +146,7 @@ def HasFunctionProto : SubsetSubject(S), false} ],
+: SubsetSubject(S), false}],
 "implicit object parameters">;
 
 // A single argument to an attribute


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

Re: r338385 - [RISCV] Add driver for riscv32-unknown-elf baremetal target

[Dávid Bolvanský via cfe-commits]

Such filename fix could be part of https://reviews.llvm.org/D50246

pi 3. 8. 2018 o 15:17 Nico Weber  napísal(a):

> I'm getting this warning from the mac linker after this commit:
>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
> warning same member name (libclangDriver.RISCV.o) in output file used for
> input files: obj/clang/lib/Driver/ToolChains/Arch/libclangDriver.RISCV.o
> and: obj/clang/lib/Driver/ToolChains/libclangDriver.RISCV.o (due to use of
> basename, truncation, blank padding or duplicate input files)
>
> Could we rename the file to fix that warning?
>
> On Tue, Jul 31, 2018 at 10:40 AM David Bolvansky via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xbolva00
>> Date: Tue Jul 31 07:21:46 2018
>> New Revision: 338385
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338385&view=rev
>> Log:
>> [RISCV] Add driver for riscv32-unknown-elf baremetal target
>>
>> Summary:
>> This patch adds a driver for the baremetal RISC-V target (i.e.
>> riscv32-unknown-elf). For reference, D39963 added basic target info and
>> added support for riscv32-linux-unknown-elf.
>>
>> Patch by: asb (Alex Bradbury)
>>
>> Reviewers: efriedma, phosek, apazos, espindola, mgrang
>>
>> Reviewed By: mgrang
>>
>> Subscribers: jrtc27, rogfer01, MartinMosbeck, brucehoult, the_o, rkruppe,
>> emaste, mgorny, arichardson, rbar, johnrusso, simoncook,
>> jordy.potman.lists, sabuasal, niosHD, kito-cheng, shiva0217, zzheng,
>> edward-jones, mgrang, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D46822
>>
>> Added:
>> cfe/trunk/lib/Driver/ToolChains/RISCV.cpp
>> cfe/trunk/lib/Driver/ToolChains/RISCV.h
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/bin/riscv32-unknown-elf-ld
>>  (with props)
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtbegin.o
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/crtend.o
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++/8.0.1/.keep
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/
>>
>> cfe/trunk/test/Driver/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib/crt0.o
>> Modified:
>> cfe/trunk/lib/Driver/CMakeLists.txt
>> cfe/trunk/lib/Driver/Driver.cpp
>> cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
>> cfe/trunk/test/Driver/riscv32-toolchain.c
>>
>> Modified: cfe/trunk/lib/Driver/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=338385&r1=338384&r2=338385&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/CMakeLists.txt (original)
>> +++ cfe/trunk/lib/Driver/CMakeLists.txt Tue Jul 31 07:21:46 2018
>> @@ -57,6 +57,7 @@ add_clang_library(clangDriver
>>ToolChains/NetBSD.cpp
>>ToolChains/OpenBSD.cpp
>>ToolChains/PS4CPU.cpp
>> +  ToolChains/RISCV.cpp
>>ToolChains/Solaris.cpp
>>ToolChains/TCE.cpp
>>ToolChains/WebAssembly.cpp
>>
>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=338385&r1=338384&r2=338385&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Jul 31 07:21:46 2018
>> @@ -37,6 +37,7 @@
>>  #include "ToolChains/NetBSD.h"
>>  #include "ToolChains/OpenBSD.h"
>>  #include "ToolChains/PS4CPU.h"
>> +#include "ToolChains/RISCV.h"
>>  #include "ToolChains/Solaris.h"
>>  #include "ToolChains/TCE.h"
>>  #include "ToolChains/WebAssembly.h"
>> @@ -4399,6 +4400,10 @@ const ToolChain &Driver::getToolChain(co
>>case llvm::Triple::avr:
>>  TC = llvm::make_unique(*this, Target,
>> Args);
>>  break;
>> +  case llvm::Triple::riscv32:
>> +  case llvm::Triple::riscv64:
>> +TC = llvm::make_unique(*this,
>> Target, Args);
>> +break;
>>default:
>>  if (Target.getVendor() == llvm::Triple::Myriad)
>>TC = llvm::make_unique(*this,
>> Target,
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
>> URL:
>> h

r338887 - [AST][NFC] Small doc update for DeclContext

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Aug  3 06:31:20 2018
New Revision: 338887

URL: http://llvm.org/viewvc/llvm-project?rev=338887&view=rev
Log:
[AST][NFC] Small doc update for DeclContext

Factored out from https://reviews.llvm.org/D49729
following @erichkeane comments.

* Add missing classes in the list of classes
  deriving directly from DeclContext.
* Move the friend declarations together and
  add a comment for why they are required.

Reviewed By: erichkeane

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

Modified:
cfe/trunk/include/clang/AST/DeclBase.h

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=338887&r1=338886&r2=338887&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Aug  3 06:31:20 2018
@@ -1250,16 +1250,29 @@ public:
 /// that directly derive from DeclContext are mentioned, not their subclasses):
 ///
 ///   TranslationUnitDecl
+///   ExternCContext
 ///   NamespaceDecl
-///   FunctionDecl
 ///   TagDecl
+///   OMPDeclareReductionDecl
+///   FunctionDecl
 ///   ObjCMethodDecl
 ///   ObjCContainerDecl
 ///   LinkageSpecDecl
 ///   ExportDecl
 ///   BlockDecl
-///   OMPDeclareReductionDecl
+///   CapturedDecl
 class DeclContext {
+  /// For makeDeclVisibleInContextImpl
+  friend class ASTDeclReader;
+  /// For reconcileExternalVisibleStorage, CreateStoredDeclsMap,
+  /// hasNeedToReconcileExternalVisibleStorage
+  friend class ExternalASTSource;
+  /// For CreateStoredDeclsMap
+  friend class DependentDiagnostic;
+  /// For hasNeedToReconcileExternalVisibleStorage,
+  /// hasLazyLocalLexicalLookups, hasLazyExternalLexicalLookups
+  friend class ASTWriter;
+
   // We use uint64_t in the bit-fields below since some bit-fields
   // cross the unsigned boundary and this breaks the packing.
 
@@ -1716,10 +1729,6 @@ protected:
   "BlockDeclBitfields is larger than 8 bytes!");
   };
 
-  friend class ASTDeclReader;
-  friend class ASTWriter;
-  friend class ExternalASTSource;
-
   /// FirstDecl - The first declaration stored within this declaration
   /// context.
   mutable Decl *FirstDecl = nullptr;
@@ -2398,8 +2407,6 @@ private:
 DeclContextBits.HasLazyExternalLexicalLookups = HasLELL;
   }
 
-  friend class DependentDiagnostic;
-
   void reconcileExternalVisibleStorage() const;
   bool LoadLexicalDeclsFromExternalStorage() const;
 


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

r338885 - Test commit

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Aug  3 06:13:05 2018
New Revision: 338885

URL: http://llvm.org/viewvc/llvm-project?rev=338885&view=rev
Log:
Test commit

Modified:
cfe/trunk/lib/AST/DeclBase.cpp

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=338885&r1=338884&r2=338885&view=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri Aug  3 06:13:05 2018
@@ -1017,7 +1017,7 @@ DeclContext::~DeclContext() = default;
 /// a friend function the parent lookup context is the lexical context, which
 /// is the class in which the friend is declared.
 DeclContext *DeclContext::getLookupParent() {
-  // FIXME: Find a better way to identify friends
+  // FIXME: Find a better way to identify friends.
   if (isa(this))
 if (getParent()->getRedeclContext()->isFileContext() &&
 getLexicalParent()->getRedeclContext()->isRecord())


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

r338888 - [AST][NFC] Add missing doc for ObjCMethodDecl and ObjCContainerDecl

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Fri Aug  3 06:47:12 2018
New Revision: 33

URL: http://llvm.org/viewvc/llvm-project?rev=33&view=rev
Log:
[AST][NFC] Add missing doc for ObjCMethodDecl and ObjCContainerDecl

Add a comment in ObjCMethodDecl and ObjCContainerDecl stating that
we store some bits in ObjCMethodDeclBits and ObjCContainerDeclBits.

This was missed by the recent move in
r338641 : [AST][4/4] Move the bit-fields from ObjCMethodDecl
  and ObCContainerDecl into DeclContext

Modified:
cfe/trunk/include/clang/AST/DeclObjC.h

Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=33&r1=338887&r2=33&view=diff
==
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Fri Aug  3 06:47:12 2018
@@ -137,6 +137,9 @@ public:
 /// the above methods are setMenu:, menu, replaceSubview:with:, and 
defaultMenu.
 ///
 class ObjCMethodDecl : public NamedDecl, public DeclContext {
+  // This class stores some data in DeclContext::ObjCMethodDeclBits
+  // to save some space. Use the provided accessors to access it.
+
 public:
   enum ImplementationControl { None, Required, Optional };
 
@@ -953,6 +956,9 @@ public:
 /// ObjCProtocolDecl, and ObjCImplDecl.
 ///
 class ObjCContainerDecl : public NamedDecl, public DeclContext {
+  // This class stores some data in DeclContext::ObjCContainerDeclBits
+  // to save some space. Use the provided accessors to access it.
+
   // These two locations in the range mark the end of the method container.
   // The first points to the '@' token, and the second to the 'end' token.
   SourceRange AtEnd;


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

[PATCH] D50193: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 159000.
kadircet marked 8 inline comments as done.
kadircet added a comment.

Fixed discussions


https://reviews.llvm.org/D50193

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Diagnostics.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -79,7 +79,7 @@
   return MemIndex::build(std::move(Slab).build());
 }
 
-CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
+CodeCompleteResult completions(ClangdServer &Server, Annotations Test,
std::vector IndexSymbols = {},
clangd::CodeCompleteOptions Opts = {}) {
   std::unique_ptr OverrideIndex;
@@ -90,23 +90,35 @@
   }
 
   auto File = testPath("foo.cpp");
-  Annotations Test(Text);
   runAddDocument(Server, File, Test.code());
   auto CompletionList =
   cantFail(runCodeComplete(Server, File, Test.point(), Opts));
   return CompletionList;
 }
 
+CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
+   std::vector IndexSymbols = {},
+   clangd::CodeCompleteOptions Opts = {}) {
+  return completions(Server, Annotations(Text), IndexSymbols, Opts);
+}
+
 // Builds a server and runs code completion.
 // If IndexSymbols is non-empty, an index will be built and passed to opts.
-CodeCompleteResult completions(StringRef Text,
+CodeCompleteResult completions(Annotations Test,
std::vector IndexSymbols = {},
clangd::CodeCompleteOptions Opts = {}) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
-  return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
+  return completions(Server, Test, std::move(IndexSymbols), std::move(Opts));
+}
+
+CodeCompleteResult completions(StringRef Text,
+   std::vector IndexSymbols = {},
+   clangd::CodeCompleteOptions Opts = {}) {
+  return completions(Annotations(Text), std::move(IndexSymbols),
+ std::move(Opts));
 }
 
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
@@ -1338,6 +1350,74 @@
   EXPECT_THAT(Results.Context, CodeCompletionContext::CCC_DotMemberAccess);
 }
 
+TEST(CompletionTest, FixItForArrowToDot) {
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+  R"cpp(
+class Auxilary {
+ public:
+  void AuxFunction();
+};
+class ClassWithPtr {
+ public:
+  void MemberFunction();
+  Auxilary* operator->() const;
+  Auxilary* Aux;
+};
+void f() {
+  ClassWithPtr x;
+  x[[->]]^;
+}
+  )cpp");
+  auto Results = completions(TestCode, {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = ".";
+  for (const auto &C : Results.Completions) {
+EXPECT_TRUE(C.FixIts.size() == 1u || C.Name == "AuxFunction");
+if (!C.FixIts.empty()) {
+  EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+}
+  }
+}
+
+TEST(CompletionTest, FixItForDotToArrow) {
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+  R"cpp(
+class Auxilary {
+ public:
+  void AuxFunction();
+};
+class ClassWithPtr {
+ public:
+  void MemberFunction();
+  Auxilary* operator->() const;
+  Auxilary* Aux;
+};
+void f() {
+  ClassWithPtr x;
+  x[[.]]^;
+}
+  )cpp");
+  auto Results = completions(TestCode, {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = "->";
+  for (const auto &C : Results.Completions) {
+EXPECT_TRUE(C.FixIts.empty() || C.Name == "AuxFunction");
+if (!C.FixIts.empty()) {
+  EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -13,6 +13,7 @@
 //===--===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
+#include "Diagnostics.h"
 #include "Protocol.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Core/

r338890 - clang-format: [JS] don't break comments before any '{'

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Aug  3 06:58:33 2018
New Revision: 338890

URL: http://llvm.org/viewvc/llvm-project?rev=338890&view=rev
Log:
clang-format: [JS] don't break comments before any '{'

Summary:
Previously, clang-format would avoid breaking before the first `{`
found, but then happily break before subsequent '{'s on the line. This
change fixes that by looking for the first location that has no opening
curly, if any.

This fixes the original commit by correcting the loop condition.

This reverts commit 66dc646e09b795b943668179c33d09da71a3b6bc.

Reviewers: krasimir

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=338890&r1=338889&r2=338890&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Fri Aug  3 06:58:33 2018
@@ -90,19 +90,21 @@ static BreakableToken::Split getCommentS
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+else
+  break;
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=338890&r1=338889&r2=338890&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Aug  3 06:58:33 2018
@@ -2067,6 +2067,14 @@ TEST_F(FormatTestJS, JSDocAnnotations) {
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",


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

[PATCH] D50249: clang-format: [JS] don't break comments before any '{'

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338890: clang-format: [JS] don't break comments before 
any '{' (authored by mprobst, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50249?vs=158993&id=159001#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50249

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


Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,21 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be 
interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that 
parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+else
+  break;
+  }
 
   if (SpaceOffset == StringRef::npos ||
   // Don't break at leading whitespace.
Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -2067,6 +2067,14 @@
" * @param {canWrap onSpace}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  // make sure clang-format doesn't break before *any* '{'
+  verifyFormat("/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   "/**\n"
+   " * @lala {lala {lalala\n"
+   " */\n",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",


Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -90,19 +90,21 @@
 
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
-  // Do not split before a number followed by a dot: this would be interpreted
-  // as a numbered list, which would prevent re-flowing in subsequent passes.
   static auto *const kNumberedListRegexp = new llvm::Regex("^[1-9][0-9]?\\.");
-  if (SpaceOffset != StringRef::npos &&
-  kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-  // In JavaScript, some @tags can be followed by {, and machinery that parses
-  // these comments will fail to understand the comment if followed by a line
-  // break. So avoid ever breaking before a {.
-  if (Style.Language == FormatStyle::LK_JavaScript &&
-  SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() &&
-  Text[SpaceOffset + 1] == '{')
-SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  while (SpaceOffset != StringRef::npos) {
+// Do not split before a number followed by a dot: this would be interpreted
+// as a numbered list, which would prevent re-flowing in subsequent passes.
+if (kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks)))
+  SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+// In JavaScript, some @tags can be followed by {, and machinery that parses
+// these comments will fail to understand the comment if followed by a line
+// break. So avoid ever breaking before a {.
+else if (Style.Language == FormatStyle::LK_JavaScript &&
+ SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+  SpaceOffset = Text.find

Re: [PATCH] D48100: Append new attributes to the end of an AttributeList.

[Aaron Ballman via cfe-commits]

On Fri, Aug 3, 2018 at 8:53 AM, Erich Keane via Phabricator
 wrote:
> erichkeane added a comment.
>
> In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:
>
>> I have two approaches to tackle the wrong marker order: 
>> https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO 
>> both are too invasive to be justified for the small issue.
>
>
> I think you're right here.  I despise https://reviews.llvm.org/D50215, and 
> only mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer leaving 
> this as a "FIXME" somewhere.

Oye, I'm in agreement that this should be fixed but that neither of
these approaches leaves me feeling warm and fuzzy.

As a possible idea (that may or may not work): the Attr object itself
has a SourceRange on it; perhaps a solution is to keep the attributes
in sorted order within DeclBase::addAttr() based on the SourceRanges
passed in?

~Aaron

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

RE: [PATCH] D48100: Append new attributes to the end of an AttributeList.

[Keane, Erich via cfe-commits]

> As a possible idea (that may or may not work): the Attr object itself has a 
> SourceRange on it; perhaps a solution is to keep the > attributes in sorted 
> order within DeclBase::addAttr() based on the SourceRanges passed in?

Interestingly, I think I came up with that idea in a comment on D50214.  I 
think that we should either keep the attributes sorted, or make the iterator 
give a sorted version.  



-Original Message-
From: Aaron Ballman [mailto:aaron.ball...@gmail.com] 
Sent: Friday, August 3, 2018 7:02 AM
To: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org
Cc: Michael Kruse ; Hal Finkel ; Tyler 
Nowicki ; Alexey Bataev ; John McCall 
; George Burgess IV ; Nick 
Lewycky ; Nick Lewycky ; d...@google.com; 
sammcc...@google.com; david.gr...@arm.com; llvm-commits 
; jrt...@jrtc27.com; Richard Smith 
; Keane, Erich ; Eric Christopher 
; zhaos...@codeaurora.org; Simon Atanasyan 
; cfe-commits ; 
junb...@codeaurora.org; florian.h...@arm.com
Subject: Re: [PATCH] D48100: Append new attributes to the end of an 
AttributeList.

On Fri, Aug 3, 2018 at 8:53 AM, Erich Keane via Phabricator 
 wrote:
> erichkeane added a comment.
>
> In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:
>
>> I have two approaches to tackle the wrong marker order: 
>> https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO 
>> both are too invasive to be justified for the small issue.
>
>
> I think you're right here.  I despise https://reviews.llvm.org/D50215, and 
> only mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer leaving 
> this as a "FIXME" somewhere.

Oye, I'm in agreement that this should be fixed but that neither of these 
approaches leaves me feeling warm and fuzzy.

As a possible idea (that may or may not work): the Attr object itself has a 
SourceRange on it; perhaps a solution is to keep the attributes in sorted order 
within DeclBase::addAttr() based on the SourceRanges passed in?

~Aaron

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

[PATCH] D49918: [clang-tidy] Sequence init statements, declarations, and conditions correctly in if, switch, and while

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked an inline comment as done.
mboehme added a comment.

Any further comments? This is marked ready to land, but I've made some 
non-trivial changes since then, so I didn't just want to land this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49918



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 159004.
lebedev.ri marked 5 inline comments as done.
lebedev.ri added a comment.

Address most of @erichkeane review notes.


Repository:
  rC Clang

https://reviews.llvm.org/D50250

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/catch-implicit-integer-conversions.c
  test/CodeGen/catch-implicit-integer-sign-changes-basics.c
  test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  test/CodeGen/catch-implicit-integer-sign-changes.c
  test/CodeGen/catch-implicit-integer-truncations.c
  test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,22 +29,22 @@
 // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation|implicit-integer-sign-change),?){7}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
-// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ???
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}} // ???
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
 
 // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS
 // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}}
Index: test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
==

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+  // We ignore conversions to/from pointer and/or bool.
+  if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+return;

erichkeane wrote:
> I'd rather !SrcType->isInt || !DestType->isInt
This i'd prefer to keep this as-is, since this is copied verbatim from 
`ScalarExprEmitter::EmitIntegerTruncationCheck()`.



Comment at: lib/CodeGen/CGExprScalar.cpp:1011
+
+  assert(isa(SrcTy) && isa(DstTy) &&
+ "clang integer type lowered to non-integer llvm type");

erichkeane wrote:
> This seems like a silly assert, since you did the check above.
If this assertion doesn't hold, we'll (hopefully!) hit an assertion somewhere 
down in the [IR] Builder.
I think it would be best to be proactive here.
(Similarly, this is copied verbatim from 
`ScalarExprEmitter::EmitIntegerTruncationCheck()`.)



Comment at: lib/CodeGen/CGExprScalar.cpp:1030
+  {
+// At least one of the values needs to have signed type.
+// If both are unsigned, then obviously, neither of them can be negative.

erichkeane wrote:
> Does this really need its own scope?
Yeah, they got out of hand here..



Comment at: lib/CodeGen/CGExprScalar.cpp:1032
+// If both are unsigned, then obviously, neither of them can be negative.
+if (!(SrcSigned || DstSigned))
+  return;

erichkeane wrote:
> Again, I'd rather we distribute the '!'.
Here - ok.



Comment at: lib/CodeGen/CGExprScalar.cpp:1044
+
+  assert(!DstType->isBooleanType() && "we should not get here with booleans.");
+

erichkeane wrote:
> Curious what prevents this?
Right now this was simply copied from 
`ScalarExprEmitter::EmitIntegerTruncationCheck()`,
where it made sense (since conversion to bool should be comparison with 0, not 
truncation).
I'm not quite sure about booleans here. I think i should just drop it, at least 
for now.



Comment at: lib/CodeGen/CGExprScalar.cpp:1054
+   const char *Name) -> Value * {
+// Does this Value has signed type?
+bool VSigned = VType->isSignedIntegerOrEnumerationType();

erichkeane wrote:
> 
> // Is this value a signed type?
That reads strange, but i don't have a better idea.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.

Are you going to propose adding this attribute to libc++, or is this expected 
to only work with UDTs?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49910



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+  // We ignore conversions to/from pointer and/or bool.
+  if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+return;

lebedev.ri wrote:
> erichkeane wrote:
> > I'd rather !SrcType->isInt || !DestType->isInt
> This i'd prefer to keep this as-is, since this is copied verbatim from 
> `ScalarExprEmitter::EmitIntegerTruncationCheck()`.
If so much is being copied from EmitIntegerTruncationCheck, perhaps the two of 
these need to be the same function with an option/check on the sanitizer option 
on which it should do?


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D50110: Handle shared release attributes correctly

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085
+  void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+  }
+
+  void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {

aaronpuchert wrote:
> aaron.ballman wrote:
> > Nothing calls either of these functions within the test; is that intended?
> Yes, I just wanted to check that there are no warnings within the functions. 
> Before the changes in `lib/Analysis/ThreadSafety.cpp`, we would get a warning 
> "releasing mutex 'mu' using shared access, expected exclusive access" on line 
> 4086.
> 
> My changes address the attributes on the function being analyzed, not on a 
> function that is called from the function being analyzed.
Ah, okay, thank you for the explanation. I wasn't aware that was previously 
failing, so I didn't see the behavioral change.


Repository:
  rC Clang

https://reviews.llvm.org/D50110



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

r338893 - revert r338831 - Fix unused variable warning in tablegen generated code

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Aug  3 07:24:34 2018
New Revision: 338893

URL: http://llvm.org/viewvc/llvm-project?rev=338893&view=rev
Log:
revert r338831 - Fix unused variable warning in tablegen generated code

No longer necessary thanks to r338889 (and friends).

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

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=338893&r1=338892&r2=338893&view=diff
==
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Fri Aug  3 07:24:34 2018
@@ -3274,15 +3274,11 @@ static std::string GenerateCustomApperta
 return "";
   }
 
-  const StringRef CheckCodeValue = Subject.getValueAsString("CheckCode");
-
   OS << "static bool " << FnName << "(const Decl *D) {\n";
-  if (CheckCodeValue != "false") {
-OS << "  if (const auto *S = dyn_cast<";
-OS << GetSubjectWithSuffix(Base);
-OS << ">(D))\n";
-OS << "return " << Subject.getValueAsString("CheckCode") << ";\n";
-  }
+  OS << "  if (const auto *S = dyn_cast<";
+  OS << GetSubjectWithSuffix(Base);
+  OS << ">(D))\n";
+  OS << "return " << Subject.getValueAsString("CheckCode") << ";\n";
   OS << "  return false;\n";
   OS << "}\n\n";
 


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

[PATCH] D49911: Summary:Add clang::reinitializes attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 159007.
mboehme marked an inline comment as done.
mboehme added a comment.

Various changes in response to review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D49911

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCXX/attr-reinitializes.cpp


Index: test/SemaCXX/attr-reinitializes.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-reinitializes.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute 
only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+  [[clang::reinitializes]] void foo();
+  __attribute__((reinitializes)) void gnu_foo();
+  [[clang::reinitializes]] void bar() const; // expected-error {{only applies 
to}}
+  [[clang::reinitializes]] static void baz(); // expected-error {{only applies 
to}}
+  [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+  [[clang::reinitializes("arg")]] void qux(); // expected-error 
{{'reinitializes' attribute takes no arguments}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6539,6 +6539,11 @@
   case ParsedAttr::AT_XRayLogArgs:
 handleXRayLogArgsAttr(S, D, AL);
 break;
+
+  // Move semantics attribute.
+  case ParsedAttr::AT_Reinitializes:
+handleSimpleAttribute(S, D, AL);
+break;
   }
 }
 
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3419,3 +3419,31 @@
 corresponding line within the inlined callee.
   }];
 }
+
+def ReinitializesDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire
+object to a known state, independent of the previous state of the object.
+
+This attribute can be interpreted by static analyzers that warn about uses of 
an
+object that has been left in an indeterminate state by a move operation. If a
+member function marked with the ``reinitializes`` attribute is called on a
+moved-from object, the analyzer can conclude that the object is no longer in an
+indeterminate state.
+
+A typical example where this attribute would be used is on functions that clear
+a container class:
+
+.. code-block:: c++
+
+  template 
+  class Container {
+  public:
+...
+[[clang::reinitializes]] void Clear();
+...
+  };
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -90,6 +90,11 @@
 [{!S->isBitField()}],
 "non-bit-field non-static data members">;
 
+def NonStaticNonConstCXXMethod
+: SubsetSubjectisStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+
 def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -2935,3 +2940,9 @@
   let Subjects = SubjectList<[Var, Function, CXXRecord]>;
   let Documentation = [InternalLinkageDocs];
 }
+
+def Reinitializes : InheritableAttr {
+  let Spellings = [Clang<"reinitializes", 0>];
+  let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
+  let Documentation = [ReinitializesDocs];
+}


Index: test/SemaCXX/attr-reinitializes.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-reinitializes.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+  [[clang::reinitializes]] void foo();
+  __attribute__((reinitializes)) void gnu_foo();
+  [[clang::reinitializes]] void bar() const; // expected-error {{only applies to}}
+  [[clang::reinitializes]] static void baz(); // expected-error {{only applies to}}
+  [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+  [[clang::reinitializes("arg")]] void qux(); // expected-error {{'reinitializes' attribute takes no arguments}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6539,6 +6539,11 @@
   case ParsedAttr::AT_XRayLogArgs:
 handleXRayLogArgsAttr(S, D, AL);
 break;
+
+  

[PATCH] D49911: Summary:Add clang::reinitializes attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 2 inline comments as done.
mboehme added a comment.

Thanks for the review!

In https://reviews.llvm.org/D49911#1186185, @aaron.ballman wrote:

> The attribute itself is looking reasonable aside from some minor nits, but 
> this should not be committed until the clang-tidy functionality is approved 
> (since it has no utility beyond clang-tidy).


Will do.




Comment at: include/clang/Basic/Attr.td:96
+[{!S->isStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+

aaron.ballman wrote:
> mboehme wrote:
> > aaron.ballman wrote:
> > > `non-static, non-const member functions` (with the comma)
> > IIUC, Clang then translates the comma into an "and" -- so the actual 
> > diagnostic becomes "non-static and non-const member functions" (see the 
> > expected-error in the tests). Is this as intended?
> Ugh, that's a deficiency in ClangAttrEmitter.cpp. Good thing diagnostics 
> aren't grammatically correct anyway, you can roll back to the previous form. 
> Sorry about the churn!
No worries!



Comment at: include/clang/Basic/Attr.td:2945
+def Reinitializes : InheritableAttr {
+  let Spellings = [CXX11<"clang", "reinitializes">];
+  let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;

aaron.ballman wrote:
> mboehme wrote:
> > aaron.ballman wrote:
> > > I think it makes sense to use `CXX11` instead of `Clang` as this 
> > > attribute doesn't make sense in C2x mode, but should there be a `GNU` 
> > > spelling as well?
> > I have to admit I'm not sure. What are the usual considerations here? Does 
> > this need to be coordinated in any way with GNU, or can Clang simply 
> > introduce additional "GNU-style" spellings (i.e. with `__attribute__`) that 
> > are Clang-only?
> > 
> > I understand there's a difference between `GCC` spellings and `GNU` 
> > spellings -- but I'm not sure what the rules around the latter are. Let me 
> > know if you think this should have a `GNU` spelling, and I'll add it!
> We generally want things with both spellings unless there's rationale 
> justifying why only one spelling is better. I don't see any reason why this 
> shouldn't have a GNU spelling as well.
> 
> I'd spell this as `Clang<"reinitializes", 0>` so it gets both the CXX11 and 
> GNU spellings but not the C2x spelling.
Thanks for the explanation -- done!

I've also added a test that the GNU spelling is recognized.


Repository:
  rC Clang

https://reviews.llvm.org/D49911



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1186460, @efriedma wrote:

> "0.0040" is four milliseconds?  You're probably crediting time incorrectly, 
> somehow.  Can you tell which FrontendTimeRAII the time is coming from?


Not sure I understand you. What do you like to know exactly? Please, clarify.


https://reviews.llvm.org/D47196



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

Re: [PATCH] D48100: Append new attributes to the end of an AttributeList.

[Aaron Ballman via cfe-commits]

On Fri, Aug 3, 2018 at 10:09 AM, Keane, Erich  wrote:
>> As a possible idea (that may or may not work): the Attr object itself has a 
>> SourceRange on it; perhaps a solution is to keep the > attributes in sorted 
>> order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> Interestingly, I think I came up with that idea in a comment on D50214.  I 
> think that we should either keep the attributes sorted, or make the iterator 
> give a sorted version.

Oh, hey, so you did! I think keeping them in a sorted order is
preferable to having the iterator return a sorted version; I believe
we iterate attribute more often than we add them because things like
hasAttr<>() and getAttr<>() both rely on iterating through the
attributes.

~Aaron

>
>
>
> -Original Message-
> From: Aaron Ballman [mailto:aaron.ball...@gmail.com]
> Sent: Friday, August 3, 2018 7:02 AM
> To: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org
> Cc: Michael Kruse ; Hal Finkel ; Tyler 
> Nowicki ; Alexey Bataev ; John 
> McCall ; George Burgess IV ; 
> Nick Lewycky ; Nick Lewycky ; 
> d...@google.com; sammcc...@google.com; david.gr...@arm.com; llvm-commits 
> ; jrt...@jrtc27.com; Richard Smith 
> ; Keane, Erich ; Eric 
> Christopher ; zhaos...@codeaurora.org; Simon Atanasyan 
> ; cfe-commits ; 
> junb...@codeaurora.org; florian.h...@arm.com
> Subject: Re: [PATCH] D48100: Append new attributes to the end of an 
> AttributeList.
>
> On Fri, Aug 3, 2018 at 8:53 AM, Erich Keane via Phabricator 
>  wrote:
>> erichkeane added a comment.
>>
>> In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:
>>
>>> I have two approaches to tackle the wrong marker order: 
>>> https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO 
>>> both are too invasive to be justified for the small issue.
>>
>>
>> I think you're right here.  I despise https://reviews.llvm.org/D50215, and 
>> only mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer 
>> leaving this as a "FIXME" somewhere.
>
> Oye, I'm in agreement that this should be fixed but that neither of these 
> approaches leaves me feeling warm and fuzzy.
>
> As a possible idea (that may or may not work): the Attr object itself has a 
> SourceRange on it; perhaps a solution is to keep the attributes in sorted 
> order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> ~Aaron
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D48100
>>
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r338895 - Fix asm label testcase flaw

[Graham Yiu via cfe-commits]

Author: gyiu
Date: Fri Aug  3 07:36:44 2018
New Revision: 338895

URL: http://llvm.org/viewvc/llvm-project?rev=338895&view=rev
Log:
Fix asm label testcase flaw

- Testcase attempts to (not) grep 'g0' in output to ensure asm symbol is
  properly renamed, but g0 is too generic and can be part of the
  module's path in LLVM IR output.
- Changed to grep '@g0', which is what the proper global symbol name
  would be if not using asm.

Modified:
cfe/trunk/test/CodeGen/2008-07-31-asm-labels.c

Modified: cfe/trunk/test/CodeGen/2008-07-31-asm-labels.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/2008-07-31-asm-labels.c?rev=338895&r1=338894&r2=338895&view=diff
==
--- cfe/trunk/test/CodeGen/2008-07-31-asm-labels.c (original)
+++ cfe/trunk/test/CodeGen/2008-07-31-asm-labels.c Fri Aug  3 07:36:44 2018
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -emit-llvm -o %t %s
 // RUN: not grep "@pipe()" %t
 // RUN: grep '_thisIsNotAPipe' %t | count 3
-// RUN: not grep 'g0' %t
+// RUN: not grep '@g0' %t
 // RUN: grep '_renamed' %t | count 2
 // RUN: %clang_cc1 -DUSE_DEF -emit-llvm -o %t %s
 // RUN: not grep "@pipe()" %t


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

[PATCH] D50255: Add test for changing build configuration

[Simon Marchi via Phabricator via cfe-commits]

simark created this revision.
Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric, ilya-biryukov.

This patch adds tests for the two ways of changing build configuration
(pointing to a particular compile_commands.json):

- Through the workspace/didChangeConfiguration notification.
- Through the initialize request.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50255

Files:
  test/clangd/compile-commands-path-in-initialize.test
  test/clangd/compile-commands-path.test


Index: test/clangd/compile-commands-path.test
===
--- /dev/null
+++ test/clangd/compile-commands-path.test
@@ -0,0 +1,37 @@
+# Test that we can switch between configuration/build using the
+# workspace/didChangeConfiguration notification.
+
+# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
+# RUN: mkdir %t.dir/build-1
+# RUN: mkdir %t.dir/build-2
+# RUN: echo '[{"directory": "%t.dir", "command": "c++ the-file.cpp", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%t.dir/build-1", "command": "c++ -DMACRO=1 
the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-1/compile_commands.json
+# RUN: echo '[{"directory": "%t.dir/build-2", "command": "c++ -DMACRO=2 
the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-2/compile_commands.json
+# RUN: sed -e "s|INPUT_DIR|%t.dir|g" %s > %t.test
+
+# RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if
 !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 
1\n#pragma message (\"MACRO is one\")\n#elif MACRO == 2\n#pragma message 
(\"MACRO is two\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() 
{}\n"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "MACRO is not defined",
+---
+{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "MACRO is one",
+---
+{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "MACRO is two",
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
Index: test/clangd/compile-commands-path-in-initialize.test
===
--- /dev/null
+++ test/clangd/compile-commands-path-in-initialize.test
@@ -0,0 +1,30 @@
+# Test that we can set choose a configuration/build directly in the initialize
+# request.
+
+# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
+# RUN: mkdir %t.dir/build-1
+# RUN: mkdir %t.dir/build-2
+# RUN: echo '[{"directory": "%t.dir", "command": "c++ the-file.cpp", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%t.dir/build-1", "command": "c++ -DMACRO=1 
the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-1/compile_commands.json
+# RUN: echo '[{"directory": "%t.dir/build-2", "command": "c++ -DMACRO=2 
the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-2/compile_commands.json
+# RUN: sed -e "s|INPUT_DIR|%t.dir|g" %s > %t.test
+
+# RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"initializationOptions":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if
 !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 
1\n#pragma message (\"MACRO is one\")\n#elif MACRO == 2\n#pragma message 
(\"MACRO is two\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() 
{}\n"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "MACRO is one",
+---
+{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "MACRO is two",
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}


Index: tes

[PATCH] D50255: Add test for changing build configuration

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

This was not tested on windows yet.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50255



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

RE: [PATCH] D48100: Append new attributes to the end of an AttributeList.

[Keane, Erich via cfe-commits]

I suspect you're right about that.  Hopefully it would be easy enough.

Unfortunately, SourceLocation doesn't have an operator<, which kind of stinks.  
I suspect that it'll be a non-trivial thing to do, since it is actually 
possible to have attributes added with empty source-locations in some case, but 
hopefully whoever is brave enough to tackle this can figure it out.

-Original Message-
From: Aaron Ballman [mailto:aaron.ball...@gmail.com] 
Sent: Friday, August 3, 2018 7:35 AM
To: Keane, Erich 
Cc: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org; Michael Kruse 
; Hal Finkel ; Tyler Nowicki 
; Alexey Bataev ; John McCall 
; George Burgess IV ; Nick 
Lewycky ; Nick Lewycky ; d...@google.com; 
sammcc...@google.com; david.gr...@arm.com; llvm-commits 
; jrt...@jrtc27.com; Richard Smith 
; Eric Christopher ; 
zhaos...@codeaurora.org; Simon Atanasyan ; cfe-commits 
; junb...@codeaurora.org; florian.h...@arm.com
Subject: Re: [PATCH] D48100: Append new attributes to the end of an 
AttributeList.

On Fri, Aug 3, 2018 at 10:09 AM, Keane, Erich  wrote:
>> As a possible idea (that may or may not work): the Attr object itself has a 
>> SourceRange on it; perhaps a solution is to keep the > attributes in sorted 
>> order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> Interestingly, I think I came up with that idea in a comment on D50214.  I 
> think that we should either keep the attributes sorted, or make the iterator 
> give a sorted version.

Oh, hey, so you did! I think keeping them in a sorted order is preferable to 
having the iterator return a sorted version; I believe we iterate attribute 
more often than we add them because things like
hasAttr<>() and getAttr<>() both rely on iterating through the attributes.

~Aaron

>
>
>
> -Original Message-
> From: Aaron Ballman [mailto:aaron.ball...@gmail.com]
> Sent: Friday, August 3, 2018 7:02 AM
> To: reviews+d48100+public+bdf72fdc7f8c9...@reviews.llvm.org
> Cc: Michael Kruse ; Hal Finkel ; 
> Tyler Nowicki ; Alexey Bataev 
> ; John McCall ; George 
> Burgess IV ; Nick Lewycky 
> ; Nick Lewycky ; d...@google.com; 
> sammcc...@google.com; david.gr...@arm.com; llvm-commits 
> ; jrt...@jrtc27.com; Richard Smith 
> ; Keane, Erich ; Eric 
> Christopher ; zhaos...@codeaurora.org; Simon 
> Atanasyan ; cfe-commits 
> ; junb...@codeaurora.org; 
> florian.h...@arm.com
> Subject: Re: [PATCH] D48100: Append new attributes to the end of an 
> AttributeList.
>
> On Fri, Aug 3, 2018 at 8:53 AM, Erich Keane via Phabricator 
>  wrote:
>> erichkeane added a comment.
>>
>> In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote:
>>
>>> I have two approaches to tackle the wrong marker order: 
>>> https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO 
>>> both are too invasive to be justified for the small issue.
>>
>>
>> I think you're right here.  I despise https://reviews.llvm.org/D50215, and 
>> only mostly hate https://reviews.llvm.org/D50216.  I think I'd prefer 
>> leaving this as a "FIXME" somewhere.
>
> Oye, I'm in agreement that this should be fixed but that neither of these 
> approaches leaves me feeling warm and fuzzy.
>
> As a possible idea (that may or may not work): the Attr object itself has a 
> SourceRange on it; perhaps a solution is to keep the attributes in sorted 
> order within DeclBase::addAttr() based on the SourceRanges passed in?
>
> ~Aaron
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D48100
>>
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In https://reviews.llvm.org/D49910#1187455, @aaron.ballman wrote:

> Are you going to propose adding this attribute to libc++, or is this expected 
> to only work with UDTs?


I don't have any experience contributing to libc++, but I think this would make 
sense.

The check currently hard-codes various member functions of classes in the "std" 
namespace that do reinitializations; I'm not sure though if those can be 
removed after the attribute has been added to libc++. We'd would also 
presumably have to add the attribute to libstdc++ -- does it accept Clang-only 
attributes? And what is the story for people using clang-tidy with MSVC 
projects? (I have to admit I'm very hazy on how that works...)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49910



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

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

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill added a comment.

@xbolva00 In my opinion this is an issue for another revision. Personally I 
would choose to rename/move the other RISCV driver file to something along the 
lines of RISCVLinux, however I think it is best for @asb to decide since he is 
the code owner for the RISC-V backend.


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D50256: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager (for == and != only)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, rnkovacs, szepet, 
whisperity.
Herald added a reviewer: george.karpenkov.

Currently, the default (range-based) constrain manager supports symbolic 
expressions of the format "symbol + integer" or "symbol - integer" to compare 
them to another integer. However, multiplication and division is not supported 
yet.

An example where multiplication (and division) could be useful is the checking 
whether a pointer operation or an array index is out of the bounds of the 
memory region. The index is often a first-degree expression of the format a*x+b 
(e.g. for special sparse such as triangular matrices).

In this patch we only support multiplication and division for two kinds of 
symbolic comparisons: the `==` and the `!=` operators. The rest is to follow in 
a subsequent patch. Negative multipliers and divisors are not supported yet.


https://reviews.llvm.org/D50256

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- /dev/null
+++ test/Analysis/multiplicative-folding.c
@@ -0,0 +1,577 @@
+// RUN: %clang_analyze_cc1 --std=c11 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (int)(UINT_MAX & (UINT_MAX >> 1))
+#define INT_MIN (-INT_MAX - 1)
+
+#define ULONG_LONG_MAX (~0UL)
+#define LONG_LONG_MAX (long long)(ULONG_LONG_MAX & (ULONG_LONG_MAX >> 1))
+#define LONG_LONG_MIN (-LONG_LONG_MAX - 1)
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef int int32_t;
+typedef unsigned int uint32_t;
+typedef long long int64_t;
+typedef unsigned long long uint64_t;
+
+void signed_multiplication_eq(int32_t n) {
+  if (n * 2 == 3) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 2 == 4) {
+const int32_t C1 = 0x8002, C2 = 2;
+
+assert(C1 * 2 == 4);
+assert(C2 * 2 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 3 == 4) {
+const int32_t C1 = 0xaaac;
+
+assert(C1 * 3 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 4 == -5) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 4 == -8) {
+const int32_t C1 = 0xbffe, C2 = 0xfffe,
+  C3 = 0x3ffe, C4 = 0x7ffe;
+
+assert(C1 * 4 == -8);
+assert(C2 * 4 == -8);
+assert(C3 * 4 == -8);
+assert(C4 * 4 == -8);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C3 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C4 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 6 == -7) {
+clang_analyzer_warnIfReached(); /

[libclc] r338898 - amdgcn: Use __constant AS for amdgcn builtins.

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Fri Aug  3 08:14:08 2018
New Revision: 338898

URL: http://llvm.org/viewvc/llvm-project?rev=338898&view=rev
Log:
amdgcn: Use __constant AS for amdgcn builtins.

Fixes build after clang r338707.
Reviewer: matthew.arsena...@amd.com
Signed-off-by: Jan Vesely 

Modified:
libclc/trunk/amdgcn-amdhsa/lib/workitem/get_global_size.cl
libclc/trunk/amdgcn-amdhsa/lib/workitem/get_local_size.cl
libclc/trunk/amdgcn/lib/workitem/get_global_offset.cl
libclc/trunk/amdgcn/lib/workitem/get_work_dim.cl

Modified: libclc/trunk/amdgcn-amdhsa/lib/workitem/get_global_size.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn-amdhsa/lib/workitem/get_global_size.cl?rev=338898&r1=338897&r2=338898&view=diff
==
--- libclc/trunk/amdgcn-amdhsa/lib/workitem/get_global_size.cl (original)
+++ libclc/trunk/amdgcn-amdhsa/lib/workitem/get_global_size.cl Fri Aug  3 
08:14:08 2018
@@ -1,6 +1,8 @@
 #include 
 
-#if __clang_major__ >= 7
+#if __clang_major__ >= 8
+#define CONST_AS __constant
+#elif __clang_major__ >= 7
 #define CONST_AS __attribute__((address_space(4)))
 #else
 #define CONST_AS __attribute__((address_space(2)))

Modified: libclc/trunk/amdgcn-amdhsa/lib/workitem/get_local_size.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn-amdhsa/lib/workitem/get_local_size.cl?rev=338898&r1=338897&r2=338898&view=diff
==
--- libclc/trunk/amdgcn-amdhsa/lib/workitem/get_local_size.cl (original)
+++ libclc/trunk/amdgcn-amdhsa/lib/workitem/get_local_size.cl Fri Aug  3 
08:14:08 2018
@@ -1,6 +1,8 @@
 #include 
 
-#if __clang_major__ >= 7
+#if __clang_major__ >= 8
+#define CONST_AS __constant
+#elif __clang_major__ >= 7
 #define CONST_AS __attribute__((address_space(4)))
 #else
 #define CONST_AS __attribute__((address_space(2)))

Modified: libclc/trunk/amdgcn/lib/workitem/get_global_offset.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn/lib/workitem/get_global_offset.cl?rev=338898&r1=338897&r2=338898&view=diff
==
--- libclc/trunk/amdgcn/lib/workitem/get_global_offset.cl (original)
+++ libclc/trunk/amdgcn/lib/workitem/get_global_offset.cl Fri Aug  3 08:14:08 
2018
@@ -1,6 +1,8 @@
 #include 
 
-#if __clang_major__ >= 7
+#if __clang_major__ >= 8
+#define CONST_AS __constant
+#elif __clang_major__ >= 7
 #define CONST_AS __attribute__((address_space(4)))
 #else
 #define CONST_AS __attribute__((address_space(2)))

Modified: libclc/trunk/amdgcn/lib/workitem/get_work_dim.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/amdgcn/lib/workitem/get_work_dim.cl?rev=338898&r1=338897&r2=338898&view=diff
==
--- libclc/trunk/amdgcn/lib/workitem/get_work_dim.cl (original)
+++ libclc/trunk/amdgcn/lib/workitem/get_work_dim.cl Fri Aug  3 08:14:08 2018
@@ -1,6 +1,8 @@
 #include 
 
-#if __clang_major__ >= 7
+#if __clang_major__ >= 8
+#define CONST_AS __constant
+#elif __clang_major__ >= 7
 #define CONST_AS __attribute__((address_space(4)))
 #else
 #define CONST_AS __attribute__((address_space(2)))


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

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 159012.
baloghadamsoftware edited the summary of this revision.
baloghadamsoftware added a comment.

Completely rewritten: works correctly for modular arithmetic (multiplication), 
works correctly for truncation (division), only uses integers, no floats.


https://reviews.llvm.org/D49074

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- test/Analysis/multiplicative-folding.c
+++ test/Analysis/multiplicative-folding.c
@@ -467,6 +467,1442 @@
   }
 }
 
+void signed_multiplication_lt_0(int32_t n) {
+  if (n * 2 < 3) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 3);
+assert(U1 * 2 < 3);
+assert((U1 + 1) * 2 >= 3);
+assert(L2 * 2 < 3);
+assert((L2 - 1) * 2 >= 3);
+assert(U2 * 2 < 3);
+assert((U2 + 1) * 2 >= 3);
+assert(L3 * 2 < 3);
+assert((L3 - 1) * 2 >= 3);
+assert(INT_MAX * 2 < 3);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_1(int32_t n) {
+  if (n * 2 < 4) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 4);
+assert(U1 * 2 < 4);
+assert((U1 + 1) * 2 >= 4);
+assert(L2 * 2 < 4);
+assert((L2 - 1) * 2 >= 4);
+assert(U2 * 2 < 4);
+assert((U2 + 1) * 2 >= 4);
+assert(L3 * 2 < 4);
+assert((L3 - 1) * 2 >= 4);
+assert(INT_MAX * 2 < 4);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_2(int32_t n) {
+  if (n * 2 < 5) {
+int32_t  U1 = 0x8002,
+L2 = 0xc000, U2 = 2,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 5);
+assert(U1 * 2 < 5);
+assert((U1 + 1) * 2 >= 5);
+assert(L2 * 2 < 5);
+assert((L2 - 1) * 2 >= 5);
+assert(U2 * 2 < 5);
+assert((U2 + 1) * 2 >= 5);
+assert(L3 * 2 < 5);
+assert((L3 - 1) * 2 >= 5);
+assert(INT_MAX * 2 < 5);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_3(int32_t n) {
+  if (n * 3 < 4) {
+int32_t  U1 = 0xaaab,
+L2 = 0xd556, U2 = 1,
+L3 = 0x2aab, U3 = 0x5556;
+
+assert(INT_MIN * 3 < 4);
+assert(U1 * 3 < 4);
+assert((U1 + 1) * 3 >= 4);
+assert(L2 * 3 < 4);
+assert((L2 - 1) * 3 >= 4);
+assert(U2 * 3 < 4);
+assert((U2 + 1) * 3 >= 4);
+assert(L3 * 3 < 4);
+assert((L3 - 1) * 3 >= 4);
+assert(U3 * 3 < 4);
+assert((U3 + 1) * 3 >= 4);
+
+if (n <

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:388
+// template parameters for different containers. So we can safely
+// assume that passing iterators of different containers as arguments
+// whose type replaces the same template parameter is a bug.

baloghadamsoftware wrote:
> a.sidorin wrote:
> > While this assumption is sane and is true for  functions, user 
> > code can have other design solutions. There is nothing that prevents users 
> > from writing a function looking like:
> > ```
> > template 
> > void f(IterTy FromBegin, IterTy FromEnd, IterTy ToBegin, IterTy ToEnd);
> > ```
> > and there is nothing wrong with it.
> > One of  possible solutions is to restrict checker to check only functions 
> > from std namespace. What do you think?
> We can restrict, of course, but first we should measure how it performs on 
> real code. With the restriction, we can get rid of some false positives but 
> we may also loose some true positives.
One more thing:

The main purpose of iterators is to make algorithms independent of the data 
representation. So you can decide whether your algorithm works on a specific 
representation and create non-template function that takes reference for the 
specific container itself or you make it generic so you use template function 
which takes iterators. If you chose this latter one for an algorithm that works 
on two different container then there is no point to restrict the function to 
only work on two containers with exactly the same representation. Either 
specific or generic, but there is no point for something in between.


https://reviews.llvm.org/D32845



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

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D32859##inline-360206, @NoQ wrote:

> I do not immediately understand what is this useful for. At least tests don't 
> look like they make use of these offset manipulations(?)
>
> Without full understanding, i wonder: when we overwrite one container with 
> another, why don't we just overwrite all symbols associated with it, instead 
> of creating a mixture of old and new symbols?
>
> Or maybe this is an accidental part of another patch, that has something to 
> do with resizes?


I do not see which lines exactly you commented but I suppose it is about not 
reassigning all iterator positions to the new container upon moving. According 
to [[ C++ Reference | en.cppreference.com/w/cpp/container/vector/operator%3D ]] 
the past-end iterators are not moved to the new container.


https://reviews.llvm.org/D32859



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

r338899 - [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

[Scott Linder via cfe-commits]

Author: scott.linder
Date: Fri Aug  3 08:50:52 2018
New Revision: 338899

URL: http://llvm.org/viewvc/llvm-project?rev=338899&view=rev
Log:
[OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

Ensures the statically sized alloca is not converted to DYNAMIC_STACKALLOC
later because it is not in the entry block.

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


Added:
cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=338899&r1=338898&r2=338899&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Aug  3 08:50:52 2018
@@ -3338,23 +3338,29 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 // Create a temporary array to hold the sizes of local pointer arguments
 // for the block. \p First is the position of the first size argument.
 auto CreateArrayForSizeVar = [=](unsigned First) {
-  auto *AT = llvm::ArrayType::get(SizeTy, NumArgs - First);
-  auto *Arr = Builder.CreateAlloca(AT);
-  llvm::Value *Ptr;
+  llvm::APInt ArraySize(32, NumArgs - First);
+  QualType SizeArrayTy = getContext().getConstantArrayType(
+  getContext().getSizeType(), ArraySize, ArrayType::Normal,
+  /*IndexTypeQuals=*/0);
+  auto Tmp = CreateMemTemp(SizeArrayTy, "block_sizes");
+  llvm::Value *TmpPtr = Tmp.getPointer();
+  llvm::Value *TmpSize = EmitLifetimeStart(
+  CGM.getDataLayout().getTypeAllocSize(Tmp.getElementType()), TmpPtr);
+  llvm::Value *ElemPtr;
   // Each of the following arguments specifies the size of the 
corresponding
   // argument passed to the enqueued block.
   auto *Zero = llvm::ConstantInt::get(IntTy, 0);
   for (unsigned I = First; I < NumArgs; ++I) {
 auto *Index = llvm::ConstantInt::get(IntTy, I - First);
-auto *GEP = Builder.CreateGEP(Arr, {Zero, Index});
+auto *GEP = Builder.CreateGEP(TmpPtr, {Zero, Index});
 if (I == First)
-  Ptr = GEP;
+  ElemPtr = GEP;
 auto *V =
 Builder.CreateZExtOrTrunc(EmitScalarExpr(E->getArg(I)), SizeTy);
 Builder.CreateAlignedStore(
 V, GEP, CGM.getDataLayout().getPrefTypeAlignment(SizeTy));
   }
-  return Ptr;
+  return std::tie(ElemPtr, TmpSize, TmpPtr);
 };
 
 // Could have events and/or varargs.
@@ -3366,24 +3372,27 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   llvm::Value *Kernel =
   Builder.CreatePointerCast(Info.Kernel, GenericVoidPtrTy);
   auto *Block = Builder.CreatePointerCast(Info.BlockArg, GenericVoidPtrTy);
-  auto *PtrToSizeArray = CreateArrayForSizeVar(4);
+  llvm::Value *ElemPtr, *TmpSize, *TmpPtr;
+  std::tie(ElemPtr, TmpSize, TmpPtr) = CreateArrayForSizeVar(4);
 
   // Create a vector of the arguments, as well as a constant value to
   // express to the runtime the number of variadic arguments.
   std::vector Args = {
   Queue,  Flags, Range,
   Kernel, Block, ConstantInt::get(IntTy, NumArgs - 4),
-  PtrToSizeArray};
+  ElemPtr};
   std::vector ArgTys = {
-  QueueTy,  IntTy,RangeTy,
-  GenericVoidPtrTy, GenericVoidPtrTy, IntTy,
-  PtrToSizeArray->getType()};
+  QueueTy,  IntTy, RangeTy,   GenericVoidPtrTy,
+  GenericVoidPtrTy, IntTy, ElemPtr->getType()};
 
   llvm::FunctionType *FTy = llvm::FunctionType::get(
   Int32Ty, llvm::ArrayRef(ArgTys), false);
-  return RValue::get(
-  Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, Name),
- llvm::ArrayRef(Args)));
+  auto Call =
+  RValue::get(Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, Name),
+ llvm::ArrayRef(Args)));
+  if (TmpSize)
+EmitLifetimeEnd(TmpSize, TmpPtr);
+  return Call;
 }
 // Any calls now have event arguments passed.
 if (NumArgs >= 7) {
@@ -3430,15 +3439,19 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   ArgTys.push_back(Int32Ty);
   Name = "__enqueue_kernel_events_varargs";
 
-  auto *PtrToSizeArray = CreateArrayForSizeVar(7);
-  Args.push_back(PtrToSizeArray);
-  ArgTys.push_back(PtrToSizeArray->getType());
+  llvm::Value *ElemPtr, *TmpSize, *TmpPtr;
+  std::tie(ElemPtr, TmpSize, TmpPtr) = CreateArrayForSizeVar(7);
+  Args.push_back(ElemPtr);
+  ArgTys.push_back(ElemPtr->getType());
 
   llvm::FunctionType *FTy = llvm::FunctionType::get(
   Int32Ty, llvm::ArrayRef(ArgTys), false);
-  return RValue::get(
-  Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, Name),

[PATCH] D50104: [OpenCL] Always emit alloca in entry block for enqueue_kernel builtin

[Scott Linder via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338899: [OpenCL] Always emit alloca in entry block for 
enqueue_kernel builtin (authored by scott.linder, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50104?vs=158816&id=159021#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50104

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl

Index: cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
===
--- cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
+++ cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple amdgcn < %s | FileCheck %s --check-prefixes=COMMON,AMDGPU
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir-unknown-unknown" < %s | FileCheck %s --check-prefixes=COMMON,SPIR32
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir64-unknown-unknown" < %s | FileCheck %s --check-prefixes=COMMON,SPIR64
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -debug-info-kind=limited -emit-llvm -o - -triple amdgcn < %s | FileCheck %s --check-prefixes=CHECK-DEBUG
+
+// Check that the enqueue_kernel array temporary is in the entry block to avoid
+// a dynamic alloca
+
+typedef struct {int a;} ndrange_t;
+
+kernel void test(int i) {
+// COMMON-LABEL: define {{.*}} void @test
+// COMMON-LABEL: entry:
+// AMDGPU: %block_sizes = alloca [1 x i64]
+// SPIR32: %block_sizes = alloca [1 x i32]
+// SPIR64: %block_sizes = alloca [1 x i64]
+// COMMON-LABEL: if.then:
+// COMMON-NOT: alloca
+// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg !34
+// COMMON-LABEL: if.end
+  queue_t default_queue;
+  unsigned flags = 0;
+  ndrange_t ndrange;
+  if (i)
+enqueue_kernel(default_queue, flags, ndrange, ^(local void *a) { }, 32);
+}
+
+// Check that the temporary is scoped to the `if`
+
+// CHECK-DEBUG: !32 = distinct !DILexicalBlock(scope: !7, file: !1, line: 24)
+// CHECK-DEBUG: !34 = !DILocation(line: 25, scope: !32)
Index: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple "spir-unknown-unknown" | FileCheck %s --check-prefix=COMMON --check-prefix=B32
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple "spir64-unknown-unknown" | FileCheck %s --check-prefix=COMMON --check-prefix=B64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O1 -emit-llvm -o - -triple "spir64-unknown-unknown" | FileCheck %s --check-prefix=CHECK-LIFETIMES
 
 #pragma OPENCL EXTENSION cl_khr_subgroups : enable
 
@@ -46,8 +47,31 @@
   // COMMON: %event_wait_list2 = alloca [1 x %opencl.clk_event_t*]
   clk_event_t event_wait_list2[] = {clk_event};
 
-  // Emits block literal on stack and block kernel [[INVLK1]].
   // COMMON: [[NDR:%[a-z0-9]+]] = alloca %struct.ndrange_t, align 4
+
+  // B32: %[[BLOCK_SIZES1:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES1:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES1:.*]] = alloca [1 x i64]
+  // B32: %[[BLOCK_SIZES2:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES2:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES2:.*]] = alloca [1 x i64]
+  // B32: %[[BLOCK_SIZES3:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES3:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES3:.*]] = alloca [1 x i64]
+  // B32: %[[BLOCK_SIZES4:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES4:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES4:.*]] = alloca [1 x i64]
+  // B32: %[[BLOCK_SIZES5:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES5:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES5:.*]] = alloca [1 x i64]
+  // B32: %[[BLOCK_SIZES6:.*]] = alloca [3 x i32]
+  // B64: %[[BLOCK_SIZES6:.*]] = alloca [3 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES6:.*]] = alloca [3 x i64]
+  // B32: %[[BLOCK_SIZES7:.*]] = alloca [1 x i32]
+  // B64: %[[BLOCK_SIZES7:.*]] = alloca [1 x i64]
+  // CHECK-LIFETIMES: %[[BLOCK_SIZES7:.*]] = alloca [1 x i64]
+
+  // Emits block literal on stack and block kernel [[INVLK1]].
   // COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
   // COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to void ()*
@@ -73,48 +97,54 @@
   // COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]],  %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.

[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic created this revision.
asavonic added reviewers: Anastasia, yaxunl.
Herald added a subscriber: cfe-commits.

Negative arguments in kernel attributes are silently bitcast'ed to
unsigned, for example:

  __attribute__((reqd_work_group_size(1, -1, 1)))
  __kernel void k() {}

is a complete equivalent of:

  __attribute__((reqd_work_group_size(1, 4294967294, 1)))
  __kernel void k() {}

This is likely an error, so the patch forbids negative arguments in
several OpenCL attributes. Users who really want 4294967294 can still
use it as an unsigned representation.

Change-Id: I910b5c077f5f29e02a1572d9202f0fdbea5280fd


Repository:
  rC Clang

https://reviews.llvm.org/D50259

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/SemaOpenCL/invalid-kernel-attrs.cl


Index: test/SemaOpenCL/invalid-kernel-attrs.cl
===
--- test/SemaOpenCL/invalid-kernel-attrs.cl
+++ test/SemaOpenCL/invalid-kernel-attrs.cl
@@ -37,3 +37,10 @@
 __attribute__((intel_reqd_sub_group_size(8))) void kernel14(){} // 
expected-error {{attribute 'intel_reqd_sub_group_size' can only be applied to 
an OpenCL kernel}}
 kernel __attribute__((intel_reqd_sub_group_size(0))) void kernel15(){} // 
expected-error {{'intel_reqd_sub_group_size' attribute must be greater than 0}}
 kernel __attribute__((intel_reqd_sub_group_size(8))) 
__attribute__((intel_reqd_sub_group_size(16))) void kernel16() {}  
//expected-warning{{attribute 'intel_reqd_sub_group_size' is already applied 
with different parameters}}
+
+__kernel __attribute__((work_group_size_hint(8,-16,32))) void neg1() {} 
//expected-error{{negative argument is not allowed for 'work_group_size_hint' 
attribute}}
+__kernel __attribute__((reqd_work_group_size(8,16,-32))) void neg2(){} // 
expected-error{{negative argument is not allowed for 'reqd_work_group_size' 
attribute}}
+
+// 4294967294 is a negative integer if treated as signed.
+// Should compile successfully, since we expect an unsigned.
+__kernel __attribute__((reqd_work_group_size(8,16,4294967294))) void ok1(){}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -239,9 +239,13 @@
 
 /// If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
+///
+/// Negative argument is implicitly converted to unsigned, unless
+/// \p StrictlyUnsigned is true.
 template 
 static bool checkUInt32Argument(Sema &S, const AttrInfo &AI, const Expr *Expr,
-uint32_t &Val, unsigned Idx = UINT_MAX) {
+uint32_t &Val, unsigned Idx = UINT_MAX,
+bool StrictlyUnsigned = false) {
   llvm::APSInt I(32);
   if (Expr->isTypeDependent() || Expr->isValueDependent() ||
   !Expr->isIntegerConstantExpr(I, S.Context)) {
@@ -262,6 +266,12 @@
 return false;
   }
 
+  if (StrictlyUnsigned && I.isSigned() && I.isNegative()) {
+S.Diag(getAttrLoc(AI), diag::err_attribute_argument_negative)
+<< getAttrName(AI);
+return false;
+  }
+
   Val = (uint32_t)I.getZExtValue();
   return true;
 }
@@ -2793,7 +2803,8 @@
   uint32_t WGSize[3];
   for (unsigned i = 0; i < 3; ++i) {
 const Expr *E = AL.getArgAsExpr(i);
-if (!checkUInt32Argument(S, AL, E, WGSize[i], i))
+if (!checkUInt32Argument(S, AL, E, WGSize[i], i,
+ /*StrictlyUnsigned=*/true))
   return;
 if (WGSize[i] == 0) {
   S.Diag(AL.getLoc(), diag::err_attribute_argument_is_zero)
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2512,6 +2512,8 @@
   "constant|a string|an identifier}1">;
 def err_attribute_argument_outof_range : Error<
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_negative : Error<
+  "negative argument is not allowed for %0 attribute">;
 def err_init_priority_object_attr : Error<
   "can only use 'init_priority' attribute on file-scope definitions "
   "of objects of class type">;


Index: test/SemaOpenCL/invalid-kernel-attrs.cl
===
--- test/SemaOpenCL/invalid-kernel-attrs.cl
+++ test/SemaOpenCL/invalid-kernel-attrs.cl
@@ -37,3 +37,10 @@
 __attribute__((intel_reqd_sub_group_size(8))) void kernel14(){} // expected-error {{attribute 'intel_reqd_sub_group_size' can only be applied to an OpenCL kernel}}
 kernel __attribute__((intel_reqd_sub_group_size(0))) void kernel15(){} // expected-error {{'intel_reqd_sub_group_size' attribute must be greater than 0}}
 kernel __attribute__((intel_reqd_sub_group_size(8))) __attribute__((intel_reqd_sub_group_size(16))) void kernel16() {}  //expected-warn

[PATCH] D49918: [clang-tidy] Sequence init statements, declarations, and conditions correctly in if, switch, and while

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

Still LG




Comment at: test/clang-tidy/bugprone-use-after-move.cpp:1141
+A a1;
+if (A a2= std::move(a1)) {
+  std::move(a2);

nit: clang-format this, please.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49918



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

[PATCH] D50193: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/CodeComplete.cpp:286
+Completion.FixIts.push_back(
+toTextEdit(FixIt, ASTCtx.getSourceManager(), {}));
+  }

IIRC LangOptions are actually important when running lexer (that is used 
internally to measure the length of the tokens).
Use `ASTCtx.getLangOptions()`?



Comment at: clangd/SourceCode.cpp:11
 
+#include "Diagnostics.h"
 #include "Logger.h"

NIT: no need for this include in .cpp file, since the header already has that.



Comment at: clangd/SourceCode.h:16
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
+#include "Diagnostics.h"
 #include "Protocol.h"

NIT: #include "clang/Basic/Diagnostic.h" should be enough here




Comment at: unittests/clangd/CodeCompleteTests.cpp:82
 
-CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
+CodeCompleteResult completions(ClangdServer &Server, Annotations Test,
std::vector IndexSymbols = {},

Maybe accept a stringref to source code and completion point directly?
A potential use-case: `Annotaions` instance might have multiple named rangers 
(e.g. if you have multiple completion points in same code and want to test one 
after another). In that case, point() will fail with an assert if we pass 
`Annotations` directly


https://reviews.llvm.org/D50193



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This revision got 'reopened' and is now in the list of accepted revision. 
Should we close it again?


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Simon Marchi via Phabricator via cfe-commits]

simark requested review of this revision.
simark added a comment.

In https://reviews.llvm.org/D48903#1187596, @ilya-biryukov wrote:

> This revision got 'reopened' and is now in the list of accepted revision. 
> Should we close it again?


It got reverted a second time because it was breaking a test on Windows.  The 
new bit is the change to `test/PCH/case-insensitive-include.c`, so it would 
need review again.  If somebody else could run the tests on Windows, it would 
make me a bit more confident too.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D50218: [OpenMP] Encode offload target triples into comdat key for offload initialization code

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev updated this revision to Diff 159028.
sdmitriev added a comment.

Replaced std::sort with llvm::sort. Added a test for offload target 
registration code for two offload targets.


https://reviews.llvm.org/D50218

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/openmp_offload_registration.cpp


Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3818,7 +3818,17 @@
 CGF.disableDebugInfo();
 const auto &FI = CGM.getTypes().arrangeNullaryFunction();
 llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);
-std::string Descriptor = getName({"omp_offloading", "descriptor_reg"});
+
+// Encode offload target triples into the registration function name. It
+// will serve as a comdat key for the registration/unregistration code for
+// this particular combination of offloading targets.
+SmallVector RegFnNameParts;
+RegFnNameParts.push_back("omp_offloading");
+RegFnNameParts.push_back("descriptor_reg");
+for (const auto &Device : Devices)
+  RegFnNameParts.push_back(Device.getTriple());
+llvm::sort(RegFnNameParts.begin() + 2, RegFnNameParts.end());
+std::string Descriptor = getName(RegFnNameParts);
 RegFn = CGM.CreateGlobalInitOrDestructFunction(FTy, Descriptor, FI);
 CGF.StartFunction(GlobalDecl(), C.VoidTy, RegFn, FI, FunctionArgList());
 CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__tgt_register_lib), Desc);
Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -0,0 +1,49 @@
+// Test for offload registration code for two targets
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu 
-fopenmp-targets=x86_64-pc-linux-gnu,powerpc64le-ibm-linux-gnu -emit-llvm %s -o 
- | FileCheck %s
+// expected-no-diagnostics
+
+void foo() {
+#pragma omp target
+  {}
+}
+
+// CHECK-DAG: [[ENTTY:%.+]] = type { i8*, i8*, i[[SZ:32|64]], i32, i32 }
+// CHECK-DAG: [[DEVTY:%.+]] = type { i8*, i8*, [[ENTTY]]*, [[ENTTY]]* }
+// CHECK-DAG: [[DSCTY:%.+]] = type { i32, [[DEVTY]]*, [[ENTTY]]*, [[ENTTY]]* }
+
+// Comdat key for the offload registration code. Should have sorted offload
+// target triples encoded into the name.
+// CHECK-DAG: 
$[[REGFN:\.omp_offloading\..+\.powerpc64le-ibm-linux-gnu\.x86_64-pc-linux-gnu+]]
 = comdat any
+
+// Check if offloading descriptor is created.
+// CHECK: [[ENTBEGIN:@.+]] = external constant [[ENTTY]]
+// CHECK: [[ENTEND:@.+]] = external constant [[ENTTY]]
+// CHECK: [[DEV1BEGIN:@.+]] = extern_weak constant i8
+// CHECK: [[DEV1END:@.+]] = extern_weak constant i8
+// CHECK: [[DEV2BEGIN:@.+]] = extern_weak constant i8
+// CHECK: [[DEV2END:@.+]] = extern_weak constant i8
+// CHECK: [[IMAGES:@.+]] = internal unnamed_addr constant [2 x [[DEVTY]]] 
[{{.+}} { i8* [[DEV1BEGIN]], i8* [[DEV1END]], [[ENTTY]]* [[ENTBEGIN]], 
[[ENTTY]]* [[ENTEND]] }, {{.+}} { i8* [[DEV2BEGIN]], i8* [[DEV2END]], 
[[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }], comdat($[[REGFN]])
+// CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* 
getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, 
i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
+
+// Check target registration is registered as a Ctor.
+// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* 
} { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+
+// Check presence of foo() and the outlined target region
+// CHECK: define void [[FOO:@.+]]()
+// CHECK: define internal void [[OUTLINEDTARGET:@.+]]() 
+
+// Check registration and unregistration code.
+
+// CHECK: define internal void @[[UNREGFN:.+]](i8*)
+// CHECK-SAME: comdat($[[REGFN]]) {
+// CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_unregister_lib([[DSCTY]]*)
+
+// CHECK: define linkonce hidden void @[[REGFN]]()
+// CHECK-SAME: comdat {
+// CHECK: call i32 @__tgt_register_lib([[DSCTY]]* [[DESC]])
+// CHECK: call i32 @__cxa_atexit(void (i8*)* @[[UNREGFN]], i8* bitcast 
([[DSCTY]]* [[DESC]] to i8*),
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_lib([[DSCTY]]*)
+


Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3818,7 +3818,17 @@
 CGF.disableDebugInfo();
 const auto &FI = CGM.getTypes().arrangeNullaryFunction();
 llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);
-std::string Descriptor = getName({"omp_offloading", "descriptor_reg"});
+
+// Encode offload target triples into the registration function name. It
+// will serve as a comdat 

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

[Bruno Ricci via Phabricator via cfe-commits]

bricci created this revision.
bricci added reviewers: erichkeane, rsmith, rnk.
bricci added a project: clang.
Herald added a subscriber: cfe-commits.

DeclarationNameTable currently hold 3 void * to
FoldingSet, FoldingSet
and FoldingSet.

CXXSpecialName,  CXXLiteralOperatorIdName and
CXXDeductionGuideNameExtra are private classes holding extra
information about a "special" declaration name and are in the 
DeclarationName.cpp
The original intent seems to have been to keep these class private and only 
expose
DeclarationNameExtra and DeclarationName (the code dates from 2008 and
has not been significantly changed since).

However this make the code less straightforward than necessary because of
the need to have void* in DeclarationNameTable (with 1 of 3 comments wrong)
and to manually allocate/deallocate the FoldingSets.

Moreover removing the extra indirections reduce the run-time of an fsyntax-only 
on
all of Boost by 2.3% which is not totally unexpected given how frequently this
data structure is used.

Comments:

- For now I just moved the classes which were in DeclarationName.cpp to 
DeclarationName.h. We could find a way to hide them if desired, eg by stuffing 
them into a namespace detail or something...
- This requires including Type.h...


Repository:
  rC Clang

https://reviews.llvm.org/D50261

Files:
  include/clang/AST/DeclarationName.h
  lib/AST/DeclarationName.cpp

Index: lib/AST/DeclarationName.cpp
===
--- lib/AST/DeclarationName.cpp
+++ lib/AST/DeclarationName.cpp
@@ -39,74 +39,6 @@
 
 using namespace clang;
 
-namespace clang {
-
-/// CXXSpecialName - Records the type associated with one of the
-/// "special" kinds of declaration names in C++, e.g., constructors,
-/// destructors, and conversion functions.
-class CXXSpecialName
-  : public DeclarationNameExtra, public llvm::FoldingSetNode {
-public:
-  /// Type - The type associated with this declaration name.
-  QualType Type;
-
-  /// FETokenInfo - Extra information associated with this declaration
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &ID) {
-ID.AddInteger(ExtraKindOrNumArgs);
-ID.AddPointer(Type.getAsOpaquePtr());
-  }
-};
-
-/// Contains extra information for the name of a C++ deduction guide.
-class CXXDeductionGuideNameExtra : public DeclarationNameExtra,
-   public llvm::FoldingSetNode {
-public:
-  /// The template named by the deduction guide.
-  TemplateDecl *Template;
-
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &ID) {
-ID.AddPointer(Template);
-  }
-};
-
-/// CXXOperatorIdName - Contains extra information for the name of an
-/// overloaded operator in C++, such as "operator+.
-class CXXOperatorIdName : public DeclarationNameExtra {
-public:
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-};
-
-/// CXXLiteralOperatorName - Contains the actual identifier that makes up the
-/// name.
-///
-/// This identifier is stored here rather than directly in DeclarationName so as
-/// to allow Objective-C selectors, which are about a million times more common,
-/// to consume minimal memory.
-class CXXLiteralOperatorIdName
-  : public DeclarationNameExtra, public llvm::FoldingSetNode {
-public:
-  IdentifierInfo *ID;
-
-  /// FETokenInfo - Extra information associated with this operator
-  /// name that can be used by the front end.
-  void *FETokenInfo;
-
-  void Profile(llvm::FoldingSetNodeID &FSID) {
-FSID.AddPointer(ID);
-  }
-};
-
-} // namespace clang
-
 static int compareInt(unsigned A, unsigned B) {
   return (A < B ? -1 : (A > B ? 1 : 0));
 }
@@ -436,10 +368,6 @@
 }
 
 DeclarationNameTable::DeclarationNameTable(const ASTContext &C) : Ctx(C) {
-  CXXSpecialNamesImpl = new llvm::FoldingSet;
-  CXXLiteralOperatorNames = new llvm::FoldingSet;
-  CXXDeductionGuideNames = new llvm::FoldingSet;
-
   // Initialize the overloaded operator names.
   CXXOperatorNames = new (Ctx) CXXOperatorIdName[NUM_OVERLOADED_OPERATORS];
   for (unsigned Op = 0; Op < NUM_OVERLOADED_OPERATORS; ++Op) {
@@ -449,21 +377,6 @@
   }
 }
 
-DeclarationNameTable::~DeclarationNameTable() {
-  auto *SpecialNames =
-  static_cast *>(CXXSpecialNamesImpl);
-  auto *LiteralNames =
-  static_cast *>(
-  CXXLiteralOperatorNames);
-  auto *DeductionGuideNames =
-  static_cast *>(
-  CXXDeductionGuideNames);
-
-  delete SpecialNames;
-  delete LiteralNames;
-  delete DeductionGuideNames;
-}
-
 DeclarationName DeclarationNameTable::getCXXConstructorName(CanQualType Ty) {
   return getCXXSpecialName(DeclarationName::CXXConstructorName,
Ty.getUnqualifiedType());
@@ -478,23 +391,19 @@
 DeclarationNameTable::getCXXDeductionGu

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a project: clang.
lebedev.ri added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+  // We ignore conversions to/from pointer and/or bool.
+  if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+return;

erichkeane wrote:
> lebedev.ri wrote:
> > erichkeane wrote:
> > > I'd rather !SrcType->isInt || !DestType->isInt
> > This i'd prefer to keep this as-is, since this is copied verbatim from 
> > `ScalarExprEmitter::EmitIntegerTruncationCheck()`.
> If so much is being copied from EmitIntegerTruncationCheck, perhaps the two 
> of these need to be the same function with an option/check on the sanitizer 
> option on which it should do?
I agree that code duplication is bad, but i'm not sure that inlining
both of these functions into an one huge one is the right solution.
The amount of actually duplicated code is somewhat small - one early-return
for non-integer types, and an assert that the llvm type is integer..


Repository:
  rC Clang

https://reviews.llvm.org/D50250



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

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

On the face, this seems to be a nice patch.  Removing extra allocations and 
replacing them with the stack is a good thing IMO. This is clearly an example 
of PIMPL, which while it has its advantages, performance is typically a hit.  
My 2 concerns with this are:

1- The increased size of DeclarationNameTable will cause a performance 
regression in the case where these folding sets are used rarely.  Copies (which 
are obviously deleted, unless memcpy'ed somewhere)/moves of 
DeclarationNameTable are now more expensive.  Based on your analysis, it seems 
that this is a 'net win', but I'd like to see if we can disallow 'move' as well.

2- The advantage of PIMPL is a reduction in compile time.  Now, every file that 
uses DeclarationName.h (an extensive list of .cpp files, but more importantly, 
Decl.h, DeclBase.h, and Sema.h).  Could you perhaps do a clean build both 
before/after this change with 'time' 
(http://man7.org/linux/man-pages/man1/time.1.html) and show the results?  I 
hope it ends up being a small enough change to be acceptable, but it would be 
nice to have an informed decision here.




Comment at: include/clang/AST/DeclarationName.h:450
   DeclarationNameTable &operator=(const DeclarationNameTable &) = delete;
 
   /// getIdentifier - Create a declaration name that is a simple

DeclarationNameTable should definitely have the move operators dealt with here. 
 I suspect/hope the answer is 'deleted', but I don't think it matters generally.

Additionally, I'd like to see the destructor line left and set to 'default', 
since those 2 would complete the 'rule of 5'.


Repository:
  rC Clang

https://reviews.llvm.org/D50261



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

[PATCH] D43953: clangformat-vs: Fix plugin not correctly loading in some cases

[Kristina Brooks via Phabricator via cfe-commits]

kristina accepted this revision as: kristina.
kristina added a comment.
This revision is now accepted and ready to land.

Seems something that's not easy to test or reproduce even with VS (the plugin 
itself is 3rd party if I understand right) but it looks sane.


Repository:
  rC Clang

https://reviews.llvm.org/D43953



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

[PATCH] D50102: [clang-tidy] Use ExprMutationAnalyzer in performance-unnecessary-value-param

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang updated this revision to Diff 159037.
shuaiwang marked 2 inline comments as done.
shuaiwang added a comment.

Add comments explaining CXXCtorInitializer check


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50102

Files:
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  test/clang-tidy/performance-unnecessary-value-param.cpp

Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -15,6 +15,20 @@
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
+template  class Vector {
+ public:
+  using iterator = T*;
+  using const_iterator = const T*;
+
+  Vector(const Vector&);
+  Vector& operator=(const Vector&);
+
+  iterator begin();
+  iterator end();
+  const_iterator begin() const;
+  const_iterator end() const;
+};
+
 // This class simulates std::pair<>. It is trivially copy constructible
 // and trivially destructible, but not trivially copy assignable.
 class SomewhatTrivial {
@@ -59,6 +73,14 @@
   useByValue(Obj);
 }
 
+void positiveVector(Vector V) {
+  // CHECK-MESSAGES: [[@LINE-1]]:49: warning: the parameter 'V' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+  // CHECK-FIXES: void positiveVector(const Vector& V) {
+  for (const auto& Obj : V) {
+useByValue(Obj);
+  }
+}
+
 void positiveWithComment(const ExpensiveToCopyType /* important */ S);
 // CHECK-FIXES: void positiveWithComment(const ExpensiveToCopyType& /* important */ S);
 void positiveWithComment(const ExpensiveToCopyType /* important */ S) {
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -10,6 +10,7 @@
 #include "UnnecessaryValueParamCheck.h"
 
 #include "../utils/DeclRefExprUtils.h"
+#include "../utils/ExprMutationAnalyzer.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/TypeTraits.h"
@@ -31,14 +32,6 @@
   .str();
 }
 
-template 
-bool isSubset(const S &SubsetCandidate, const S &SupersetCandidate) {
-  for (const auto &E : SubsetCandidate)
-if (SupersetCandidate.count(E) == 0)
-  return false;
-  return true;
-}
-
 bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
ASTContext &Context) {
   auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
@@ -98,43 +91,55 @@
 void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Param = Result.Nodes.getNodeAs("param");
   const auto *Function = Result.Nodes.getNodeAs("functionDecl");
-  const size_t Index = std::find(Function->parameters().begin(),
- Function->parameters().end(), Param) -
-   Function->parameters().begin();
-  bool IsConstQualified =
-  Param->getType().getCanonicalType().isConstQualified();
 
-  auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs(
-  *Param, *Function, *Result.Context);
-  auto ConstDeclRefExprs = utils::decl_ref_expr::constReferenceDeclRefExprs(
-  *Param, *Function, *Result.Context);
-
-  // Do not trigger on non-const value parameters when they are not only used as
-  // const.
-  if (!isSubset(AllDeclRefExprs, ConstDeclRefExprs))
+  // Do not trigger on non-const value parameters when they are mutated either
+  // within the function body or within init expression(s) when the function is
+  // a ctor.
+  if (utils::ExprMutationAnalyzer(Function->getBody(), Result.Context)
+  .isMutated(Param))
 return;
+  // CXXCtorInitializer might also mutate Param but they're not part of function
+  // body, so check them separately here.
+  if (const auto *Ctor = dyn_cast(Function)) {
+for (const auto *Init : Ctor->inits()) {
+  if (utils::ExprMutationAnalyzer(Init->getInit(), Result.Context)
+  .isMutated(Param))
+return;
+}
+  }
+
+  const bool IsConstQualified =
+  Param->getType().getCanonicalType().isConstQualified();
 
   // If the parameter is non-const, check if it has a move constructor and is
   // only referenced once to copy-construct another object or whether it has a
   // move assignment operator and is only referenced once when copy-assigned.
   // In this case wrap DeclRefExpr with std::move() to avoid the unnecessary
   // copy.
-  if (!IsConstQualified && AllDeclRefExprs.size() == 1) {
-auto CanonicalType = Param->getType().getCanonicalType();
-const auto &DeclRefExpr  = **AllDeclRefExprs.begin();
-
-if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
-((utils::type_trait

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 159036.
arphaman added a comment.

Always capitalize the diagnostic's message.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50154

Files:
  clangd/Diagnostics.cpp
  test/clangd/capitalize-diagnostics.test
  test/clangd/diagnostics.test
  test/clangd/did-change-configuration-params.test
  test/clangd/execute-command.test
  test/clangd/extra-flags.test
  test/clangd/fixits.test

Index: test/clangd/fixits.test
===
--- test/clangd/fixits.test
+++ test/clangd/fixits.test
@@ -6,7 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"message": "using the result of an assignment as a condition without parentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 37,
@@ -23,7 +23,7 @@
 # CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses"}]}}}
 #  CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
@@ -92,7 +92,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 ---
-{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"}]}}}
+{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses"}]}}}
 # Make sure unused "code" and "source" fields ignored gracefully
 #  CHECK:  "id": 3,
 # CHECK-NEXT:  "jsonrpc": "2.0",
Index: test/clangd/extra-flags.test
===
--- test/clangd/extra-flags.test
+++ test/clangd/extra-flags.test
@@ -6,7 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 28,
@@ -28,7 +28,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 28,
Index: test/clangd/execute-command.test
===
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -6,7 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"message": "using the result of an assignment as a condition without parentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 37,
Index: test/clangd/did-change-configuration-params.test
===
--- test/clangd/did-change-configuration-params.test
+++ test/clangd/did-change-configuration-params.test
@@ -24,7 +24,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT: