[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This isn't a general fix to all paths where we assume case-sensitivity, it's
a minimally-invasive fix targeting the llvm 9 branch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65320

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -80,7 +80,8 @@
   llvm::Optional getProjectInfo(PathRef File) const override;
 
 private:
-  std::pair
+  std::tuple
   getCDBInDirLocked(PathRef File) const;
 
   struct CDBLookupRequest {
@@ -101,9 +102,11 @@
   /// Caches compilation databases loaded from directories(keys are
   /// directories).
   struct CachedCDB {
+std::string Path; // Not case-folded.
 std::unique_ptr CDB = nullptr;
 bool SentBroadcast = false;
   };
+  // Keyed by possibly-case-folded path!
   mutable llvm::StringMap CompilationDatabases;
 
   /// Used for command argument pointing to folder where compile_commands.json
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -117,20 +117,48 @@
   return None;
 }
 
-std::pair
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+//This is not the only place this problem exists.
+// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
+
+static std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return Path;
+#endif
+}
+
+static bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
+
+// If a CDB is returned, *CanonDir is it's original, non-case-folded directory.
+std::tuple
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
-  auto CachedIt = CompilationDatabases.find(Dir);
-  if (CachedIt != CompilationDatabases.end())
-return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
+  auto Key = maybeCaseFoldPath(Dir);
+  auto CachedIt = CompilationDatabases.find(Key);
+  if (CachedIt != CompilationDatabases.end()) {
+return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast,
+CachedIt->second.Path};
+  }
   std::string Error = "";
 
   CachedCDB Entry;
   Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+  Entry.Path = Dir;
   auto Result = Entry.CDB.get();
-  CompilationDatabases[Dir] = std::move(Entry);
+  CompilationDatabases[Key] = std::move(Entry);
 
-  return {Result, false};
+  return {Result, false, Dir};
 }
 
 llvm::Optional
@@ -145,17 +173,16 @@
   {
 std::lock_guard Lock(Mutex);
 if (CompileCommandsDir) {
-  std::tie(Result.CDB, SentBroadcast) =
+  std::tie(Result.CDB, SentBroadcast, Result.PI.SourceRoot) =
   getCDBInDirLocked(*CompileCommandsDir);
-  Result.PI.SourceRoot = *CompileCommandsDir;
 } else {
   // Traverse the canonical version to prevent false positives. i.e.:
   // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
   actOnAllParentDirectories(removeDots(Request.FileName),
 [this, &SentBroadcast, &Result](PathRef Path) {
-  std::tie(Result.CDB, SentBroadcast) =
+  std::tie(Result.CDB, SentBroadcast,
+   Result.PI.SourceRoot) =
   getCDBInDirLocked(Path);
-  Result.PI.SourceRoot = Path;
   return Result.CDB != nullptr;
 });
 }
@@ -201,8 +228,8 @@
   return true;
 
 auto Res = getCDBInDirLocked(Path);
-It.first->second = Res.first != nullptr;
-return Path == Result.PI.SourceRoot;
+It.first->second = std::get<0>(Res) != nullptr;
+return pathEqual(Path, Result.PI.SourceRoot);
   });
 }
   }
@@ -213,7 +240,7 @@
 // Independent of whether it has an entry for that file or not.
 actOnAllParentDirectories(File, [&](PathRef Path) {
   if (DirectoryHasCDB.lookup(Path)) {
-if (Path == Result.PI.SourceRoot)
+if (pathEqual(Path, Resul

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 211890.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Made tests more readable. Updated to work with rL366577 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -29,9 +29,7 @@
   return Tokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
-  Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+std::vector getExpectedTokens(Annotations &Test) {
   static const std::map KindToString{
   {HighlightingKind::Variable, "Variable"},
   {HighlightingKind::Function, "Function"},
@@ -48,10 +46,42 @@
 Test.ranges(KindString.second), KindString.first);
 ExpectedTokens.insert(ExpectedTokens.end(), Toks.begin(), Toks.end());
   }
+  llvm::sort(ExpectedTokens);
+  return ExpectedTokens;
+}
 
-  auto ActualTokens = getSemanticHighlightings(AST);
-  EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens))
-   << "Inputs is:\n" << Code;
+void checkHighlightings(llvm::StringRef Code) {
+  Annotations Test(Code);
+  auto AST = TestTU::withCode(Test.code()).build();
+  std::vector ActualTokens = getSemanticHighlightings(AST);
+  EXPECT_THAT(ActualTokens, getExpectedTokens(Test));
+}
+
+void checkDiffedHighlights(llvm::StringRef OldCode, llvm::StringRef NewCode) {
+  Annotations OldTest(OldCode);
+  Annotations NewTest(NewCode);
+  std::vector OldTokens = getExpectedTokens(OldTest);
+  std::vector NewTokens = getExpectedTokens(NewTest);
+
+  std::map> ExpectedLines;
+  for (const Position &Point : NewTest.points()) {
+ExpectedLines[Point.line]; // Default initialize to an empty line. Tokens
+   // are inserted on these lines later.
+  }
+  std::vector ExpectedLinePairHighlighting;
+  for (const HighlightingToken &Token : NewTokens) {
+auto It = ExpectedLines.find(Token.R.start.line);
+if (It != ExpectedLines.end())
+  It->second.push_back(Token);
+  }
+  for (auto &LineTokens : ExpectedLines)
+ExpectedLinePairHighlighting.push_back(
+{LineTokens.first, LineTokens.second});
+
+  std::vector ActualDiffed =
+  diffHighlightings(NewTokens, OldTokens, NewCode.count('\n'));
+  EXPECT_THAT(ActualDiffed,
+  testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting));
 }
 
 TEST(SemanticHighlighting, GetsCorrectTokens) {
@@ -226,8 +256,9 @@
 std::atomic Count = {0};
 
 void onDiagnosticsReady(PathRef, std::vector) override {}
-void onHighlightingsReady(
-PathRef File, std::vector Highlightings) override {
+void onHighlightingsReady(PathRef File,
+  std::vector Highlightings,
+  int NLines) override {
   ++Count;
 }
   };
@@ -252,21 +283,126 @@
 return Pos;
   };
 
-  std::vector Tokens{
-  {HighlightingKind::Variable,
-Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
-  {HighlightingKind::Function,
-Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
-  {HighlightingKind::Variable,
-Range{CreatePosition(1, 1), CreatePosition(1, 5)}}};
+  std::vector Tokens{
+  {3,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+{HighlightingKind::Function,
+ Range{CreatePosition(3, 4), CreatePosition(3, 7),
+  {1,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
-  {1, "AQAEAAA="},
-  {3, "CAAEAAAEAAMAAQ=="}};
+  {3, "CAAEAAAEAAMAAQ=="}, {1, "AQAEAAA="}};
   EXPECT_EQ(ActualResults, ExpectedResults);
 }
 
+TEST(SemanticHighlighting, HighlightingDiffer) {
+  struct {
+llvm::StringRef OldCode;
+llvm::StringRef NewCode;
+  } TestCases[]{
+{
+  R"(
+$Variable[[A]]
+$Class[[B]]
+$Function[[C]]
+  )",
+  R"(
+$Variable[[A]]
+$Class[[D]]
+$Function[[C]]
+  )"},
+{
+  R"(
+$Class[[C]]
+ 

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added a comment.

In D64475#1593481 , @ilya-biryukov 
wrote:

> The fix for a race condition on remove has landed in rL366577 
> , this revision would need a small update 
> after it.


Fixed to work with that patch.
Should the diffing be moved somewhere else that is not inside the publish 
function though? That would require moving the state outside the LSP server 
though and handle the onClose/onOpen events somewhere else though. 
Maybe it's ok to keep the diffing in the publish lock?




Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:307
+llvm::StringRef NewCode;
+std::vector DiffedLines;
+  } TestCases[]{

ilya-biryukov wrote:
> @hokein rightfully pointed out that mentioning all changed lines makes the 
> tests unreadable.
> An alternative idea we all came up with is to force people to put `^` on each 
> of the changed lines inside the `NewCode`, i.e.
> 
> ```
> {/*Before*/ R"(
>   $Var[[a]]
>   $Func[[b]]
> "),
>  /*After*/ R"(
>   $Var[[a]]
>  ^$Func[[b]]
> )"} // and no list of lines is needed!
> ```
> 
> Could we do that here?
> 
> One interesting case that we can't test this way to removing lines from the 
> end of the file. But for that particular case, could we just write a separate 
> test case?
We don't want to send diffs that are beyond the end of the file. There is a 
testcase for that as well (the count of newlines in the new code is sent to the 
differ as the number of lines in the file).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:50
   // Generate Replacement for declaring the selected Expr as a new variable
-  tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+  tooling::Replacement insertDeclaration(llvm::StringRef VarName,
+ SourceRange InitChars) const;

sammccall wrote:
> SureYeaah wrote:
> > Nit: same parameter order for replaceWithVar and insertDeclaration.
> I think this *is* the same parameter order semantically: this is roughly 
> (thing, definition) in both cases.
> The fact that the *types* match but in the opposite order is just a 
> coincidence I think?
Sorry yes, makes sense.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:305
+  const SelectionTree::Node *Start = Op.SelectedOperands.front(); // LHS
+  const SelectionTree::Node *End = Op.SelectedOperands.back();// RHS
+  // End is already correct, Start needs to be pushed down to the right spot.

sammccall wrote:
> SureYeaah wrote:
> > For 1 + [[2 + 3 * 4]] + 5, we probably don't get an invalid range from this 
> > function. If so, we might want to add one more check where we parse End and 
> > compare it's operator.
> > 
> > 
> I'm not quite sure what you mean here.
> 
> Annotating the operators, for `1 +a [[2 +b 3 *c 4]] +d 5`:
>  - N is `*c`
>  - RHS is `4`
>  - LHS is `1 +a 2 +b 3 *c 4`.
> 
> The point is that RHS can never be an operator of the same type, because if 
> we replaced RHS with `x +e y` then N would be that `+e` instead (since `+` is 
> left-associative).
> 
But isn't the tree something like

+
 /\ 
+ 5
 / \
 +   *
   /  \   /   \
   1234


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65139



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

[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:448
+
+  // Binary subexpressions
+  {R"cpp(void f() {

sammccall wrote:
> SureYeaah wrote:
> > Can we have some tests with macros as well?
> Added a simple one that just verifies we support operands being wrapped in 
> macros, but not operators.
Isn't it the same behavior in both cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65139



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

[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:305
+  const SelectionTree::Node *Start = Op.SelectedOperands.front(); // LHS
+  const SelectionTree::Node *End = Op.SelectedOperands.back();// RHS
+  // End is already correct, Start needs to be pushed down to the right spot.

SureYeaah wrote:
> sammccall wrote:
> > SureYeaah wrote:
> > > For 1 + [[2 + 3 * 4]] + 5, we probably don't get an invalid range from 
> > > this function. If so, we might want to add one more check where we parse 
> > > End and compare it's operator.
> > > 
> > > 
> > I'm not quite sure what you mean here.
> > 
> > Annotating the operators, for `1 +a [[2 +b 3 *c 4]] +d 5`:
> >  - N is `*c`
> >  - RHS is `4`
> >  - LHS is `1 +a 2 +b 3 *c 4`.
> > 
> > The point is that RHS can never be an operator of the same type, because if 
> > we replaced RHS with `x +e y` then N would be that `+e` instead (since `+` 
> > is left-associative).
> > 
> But isn't the tree something like
> 
> +
>  /\ 
> + 5
>  / \
>  +   *
>/  \   /   \
>1234
Whoops, somehow I transcribed that without noticing c was a * instead of a +.

From offline discussion: the code is apparently correct here: RHS can't be a 
matching operator, because it violates associativity: the parse tree would be 
rotated in that case. We don't care to distinguish between mismatching operator 
vs some other node entirely.
- Expanded a comment to cover this more explicitly
- adjusted a testcase to ensure that we expand a `+`-selection across a `*` on 
the RHS as well as the LHS.

There's also the idea that it would be nice to verify that we don't bail out of 
this function just because the selection covers some mismatched operator. The 
idea of a test that we keep digging right if left mismatches and vice versa is 
appealing, but as described we never need to to dig right, so such a case 
doesn't exist. I've settled for ensuring that 0 + 1 * [2 + 3] * 4 + 5 expands 
to cover 1 and 4, but not 0 or 5.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65139



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

[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 211905.
sammccall added a comment.

Improve testcase, expand comment to clarify RHS non-traversal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65139

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -444,6 +444,77 @@
   // FIXME: Doesn't work correctly for \[\[clang::uninitialized\]\] int
   // b = [[1]]; since the attr is inside the DeclStmt and the bounds of
   // DeclStmt don't cover the attribute
+
+  // Binary subexpressions
+  {R"cpp(void f() {
+   int x = 1 + [[2 + 3 + 4]] + 5;
+ })cpp",
+   R"cpp(void f() {
+   auto dummy = 2 + 3 + 4; int x = 1 + dummy + 5;
+ })cpp"},
+  {R"cpp(void f() {
+   int x = [[1 + 2 + 3]] + 4 + 5;
+ })cpp",
+   R"cpp(void f() {
+   auto dummy = 1 + 2 + 3; int x = dummy + 4 + 5;
+ })cpp"},
+  {R"cpp(void f() {
+   int x = 1 + 2 + [[3 + 4 + 5]];
+ })cpp",
+   R"cpp(void f() {
+   auto dummy = 3 + 4 + 5; int x = 1 + 2 + dummy;
+ })cpp"},
+  // Non-associative operations have no special support
+  {R"cpp(void f() {
+   int x = 1 - [[2 - 3 - 4]] - 5;
+ })cpp",
+   R"cpp(void f() {
+   auto dummy = 1 - 2 - 3 - 4; int x = dummy - 5;
+ })cpp"},
+  // A mix of associative operators isn't associative.
+  {R"cpp(void f() {
+   int x = 0 + 1 * [[2 + 3]] * 4 + 5;
+ })cpp",
+   R"cpp(void f() {
+   auto dummy = 1 * 2 + 3 * 4; int x = 0 + dummy + 5;
+ })cpp"},
+  // Overloaded operators are supported, we assume associativity
+  // as if they were built-in.
+  {R"cpp(struct S {
+   S(int);
+ };
+ S operator+(S, S);
+
+ void f() {
+   S x = S(1) + [[S(2) + S(3) + S(4)]] + S(5);
+ })cpp",
+   R"cpp(struct S {
+   S(int);
+ };
+ S operator+(S, S);
+
+ void f() {
+   auto dummy = S(2) + S(3) + S(4); S x = S(1) + dummy + S(5);
+ })cpp"},
+   // Don't try to analyze across macro boundaries
+   // FIXME: it'd be nice to do this someday (in a safe way)
+  {R"cpp(#define ECHO(X) X
+ void f() {
+   int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
+ })cpp",
+   R"cpp(#define ECHO(X) X
+ void f() {
+   auto dummy = 1 + ECHO(2 + 3) + 4; int x = dummy + 5;
+ })cpp"},
+  {R"cpp(#define ECHO(X) X
+ void f() {
+   int x = 1 + [[ECHO(2) + ECHO(3) + 4]] + 5;
+ })cpp",
+   R"cpp(#define ECHO(X) X
+ void f() {
+   auto dummy = 1 + ECHO(2) + ECHO(3) + 4; int x = dummy + 5;
+ })cpp"},
+
   };
   for (const auto &IO : InputOutputs) {
 checkTransform(ID, IO.first, IO.second);
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -335,6 +335,23 @@
   }
 }
 
+TEST(SelectionTest, Implicit) {
+  const char* Test = R"cpp(
+struct S { S(const char*); };
+int f(S);
+int x = f("^");
+  )cpp";
+  auto AST = TestTU::withCode(Annotations(Test).code()).build();
+  auto T = makeSelectionTree(Test, AST);
+
+  const SelectionTree::Node *Str = T.commonAncestor();
+  EXPECT_EQ("StringLiteral", nodeKind(Str)) << "Implicit selected?";
+  EXPECT_EQ("ImplicitCastExpr", nodeKind(Str->Parent));
+  EXPECT_EQ("CXXConstructExpr", nodeKind(Str->Parent->Parent));
+  EXPECT_EQ(Str, &Str->Parent->Parent->ignoreImplicit())
+  << "Didn't unwrap " << nodeKind(&Str->Parent->Parent->ignoreImplicit());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/cl

[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via Phabricator via cfe-commits]

sammccall planned changes to this revision.
sammccall marked an inline comment as done.
sammccall added a comment.

Sorry, this has a hideous bug that rebroadcasts CDBs over and over. Working on 
a fix...




Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:195
 if (Request.ShouldBroadcast && !SentBroadcast)
   CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
   }

this line is incorrect because the key should be lowercase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65320



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

[PATCH] D65294: Make the CXXABIs respect the target's default calling convention.

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

This should probably go to release 9.0?


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

https://reviews.llvm.org/D65294



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D65286#1601404 , @rjmccall wrote:

> Wait, plain C++?  Do we currently allow this syntax outside of OpenCL?


Ok I looked into this again and it seems apparently vector initialization and 
literals in C++ have different syntax. So I was incorrect, this fix only works 
for OpenCL. Not sure how important it is to initialize vectors from other 
vectors in C++ though. In OpenCL C it is common.


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

https://reviews.llvm.org/D65286



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as done.
glider added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:1989
   std::vector Args;
+  std::vector ResultTypeRequiresCast;
 

nickdesaulniers wrote:
> Are we able to use something other than `std::vector` here from ADT?
> http://llvm.org/docs/ProgrammersManual.html#bit-storage-containers-bitvector-sparsebitvector
I don't think `std::vector` is a bottleneck here, but since it might be 
deprecated someday let's just not use it.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2287
   assert(RegResults.size() == ResultRegDests.size());
+  assert(ResultTypeRequiresCast.size() <= ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {

efriedma wrote:
> Not "=="?
Turns out ResultRegDests can be also extended by `addReturnRegisterOutputs` 
above (line 2122), so no.
I've added a comment to clarify that.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2325
+  Dest = MakeAddrLValue(
+  A, getContext().getIntTypeForBitwidth(Size, /*Signed*/ false));
+}

efriedma wrote:
> Will this work if the struct is an unusual size, like `sizeof(struct s) == 3` 
> or `sizeof(struct s) == 32`?  (3 is unlikely to show up in real code, but 32 
> could correspond to a vector register.)
For a 3-byte struct this works as follows: a 3-byte structure is allocated on 
the stack with alignment 2, then the assembly returns a 4-byte integer that's 
written to that 3-byte structure.
This conforms to what GCC does, and I think it's legal to let the assembly 
write more than a structure size here, as the corresponding register size is 
bigger (i.e. it's a bug in the C code, not Clang).

For a 32-bit struct we crash now, whereas GCC reports an "impossible constraint 
in ‘asm’"
It's interesting that the crash happens in the frontend, i.e. it's somehow 
knows the maximum possible size for the register operand.
We can check that `getContext().getIntTypeForBitwidth(Size, /*Signed*/ false)` 
is a nonnull type to prevent the crashes.



Comment at: clang/test/CodeGen/PR42672.c:50
+  struct {
+long long int v1, v2, v3, v4;
+  } str;

The acceptable size actually depends on the target platform. Not sure we can 
test for all of them, and we'll probably need to restrict this test to e.g. x86


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65294: Make the CXXABIs respect the target's default calling convention.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Committed, 9.0 release request here: https://bugs.llvm.org/show_bug.cgi?id=42774


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

https://reviews.llvm.org/D65294



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

r367103 - Make the CXXABIs respect the target's default calling convention.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Jul 26 05:36:12 2019
New Revision: 367103

URL: http://llvm.org/viewvc/llvm-project?rev=367103&view=rev
Log:
Make the CXXABIs respect the target's default calling convention.

SPIR targets need to have all functions be SPIR calling convention,
however the CXXABIs were just returning CC_C in all non-'this-CC' cases.

https://reviews.llvm.org/D65294

Modified:
cfe/trunk/lib/AST/ItaniumCXXABI.cpp
cfe/trunk/lib/AST/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-with-class.cl
cfe/trunk/test/CodeGenOpenCLCXX/method-overload-address-space.cl
cfe/trunk/test/CodeGenOpenCLCXX/template-address-spaces.cl

Modified: cfe/trunk/lib/AST/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumCXXABI.cpp?rev=367103&r1=367102&r2=367103&view=diff
==
--- cfe/trunk/lib/AST/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumCXXABI.cpp Fri Jul 26 05:36:12 2019
@@ -177,7 +177,7 @@ public:
 if (!isVariadic && T.isWindowsGNUEnvironment() &&
 T.getArch() == llvm::Triple::x86)
   return CC_X86ThisCall;
-return CC_C;
+return Context.getTargetInfo().getDefaultCallingConv();
   }
 
   // We cheat and just check that the class has a vtable pointer, and that it's

Modified: cfe/trunk/lib/AST/MicrosoftCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=367103&r1=367102&r2=367103&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftCXXABI.cpp Fri Jul 26 05:36:12 2019
@@ -82,7 +82,7 @@ public:
 if (!isVariadic &&
 Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
   return CC_X86ThisCall;
-return CC_C;
+return Context.getTargetInfo().getDefaultCallingConv();
   }
 
   bool isNearlyEmpty(const CXXRecordDecl *RD) const override {

Modified: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl?rev=367103&r1=367102&r2=367103&view=diff
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl (original)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl Fri Jul 26 
05:36:12 2019
@@ -12,11 +12,11 @@ public:
 void foo() {
   D d;
   //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
-  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  //CHECK: call spir_func i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
   d.getmb();
 }
 
 //Derived and Base are in the same address space.
 
-//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* 
%this)
+//CHECK: define linkonce_odr spir_func i32 @_ZNU3AS41D5getmbEv(%class.D 
addrspace(4)* %this)
 //CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*

Modified: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl?rev=367103&r1=367102&r2=367103&view=diff
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl (original)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl Fri Jul 26 05:36:12 
2019
@@ -81,32 +81,32 @@ __kernel void test__global() {
 // COMMON: @_ZNU3AS41C7outsideEv(%class.C addrspace(4)* %this)
 
 // EXPL-LABEL: @__cxx_global_var_init()
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(1)* @c to %class.C addrspace(4)*))
+// EXPL: call spir_func void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
 // COMMON-LABEL: @test__global()
 
 // Test the address space of 'this' when invoking a method.
-// COMMON: call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(1)* @c to %class.C addrspace(4)*))
+// COMMON: call spir_func i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 // Test the address space of 'this' when invoking a method using a pointer to 
the object.
-// COMMON: call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(1)* @c to %class.C addrspace(4)*))
+// COMMON: call spir_func i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method that is declared in 
the file contex.
-// COMMON: call i32 @_ZNU3AS41C7outsideEv(%class.C addrspace(4)* addrsp

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 211919.
glider marked 2 inline comments as done.
glider added a comment.

Fixed comments from Eli and Nick, added tests for unusual struct sizes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -DIMPOSSIBLE -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles sizes for which no direct register conversion is possible.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2273,6 +2284,9 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  // ResultRegDests can be also populated by addReturnRegis

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 211920.
glider added a comment.

Make big_struct() test triple-specific


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/PR42672.c
  clang/test/CodeGen/asm-attrs.c

Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/test/CodeGen/PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/PR42672.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles sizes for which no direct register conversion is possible.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+ResultRegTypes.push_back(ResultTruncRegTypes.back());
+  }
   // If this output is tied to an input, and if the input is larger, then
   // we need to set the actual result type of the inline asm node to be the
   // same as the input type.
@@ -2273,6 +2284,9 @@
   assert(RegResults.size() == ResultRegTypes.size());
   assert(RegResults.size() == ResultTruncRegTypes.size());
   assert(RegResults.size() == ResultRegDests.size());
+  // ResultRegDests can be also populated by addReturnRegisterOutputs() above,
+  // in which case 

[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 211921.
sammccall added a comment.

Fix bug where CDB wasn't being marked as broadcast due to case differences.
Having getCDBLocked() return CachedCDB seems much cleaner to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65320

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -80,8 +80,13 @@
   llvm::Optional getProjectInfo(PathRef File) const override;
 
 private:
-  std::pair
-  getCDBInDirLocked(PathRef File) const;
+  /// Caches compilation databases loaded from directories.
+  struct CachedCDB {
+std::string Path; // Not case-folded.
+std::unique_ptr CDB = nullptr;
+bool SentBroadcast = false;
+  };
+  CachedCDB &getCDBInDirLocked(PathRef File) const;
 
   struct CDBLookupRequest {
 PathRef FileName;
@@ -98,12 +103,7 @@
   void broadcastCDB(CDBLookupResult Res) const;
 
   mutable std::mutex Mutex;
-  /// Caches compilation databases loaded from directories(keys are
-  /// directories).
-  struct CachedCDB {
-std::unique_ptr CDB = nullptr;
-bool SentBroadcast = false;
-  };
+  // Keyed by possibly-case-folded directory path.
   mutable llvm::StringMap CompilationDatabases;
 
   /// Used for command argument pointing to folder where compile_commands.json
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -117,20 +117,41 @@
   return None;
 }
 
-std::pair
-DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
-  // FIXME(ibiryukov): Invalidate cached compilation databases on changes
-  auto CachedIt = CompilationDatabases.find(Dir);
-  if (CachedIt != CompilationDatabases.end())
-return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
-  std::string Error = "";
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+//This class is not the only place this problem exists.
+// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
+
+static std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return Path;
+#endif
+}
 
-  CachedCDB Entry;
-  Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
-  auto Result = Entry.CDB.get();
-  CompilationDatabases[Dir] = std::move(Entry);
+static bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
 
-  return {Result, false};
+DirectoryBasedGlobalCompilationDatabase::CachedCDB &
+DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
+  // FIXME(ibiryukov): Invalidate cached compilation databases on changes
+  // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
+  auto Key = maybeCaseFoldPath(Dir);
+  auto R = CompilationDatabases.try_emplace(Key);
+  if (R.second) { // Cache miss, try to load CDB.
+CachedCDB &Entry = R.first->second;
+std::string Error = "";
+Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+Entry.Path = Dir;
+  }
+  return R.first->second;
 }
 
 llvm::Optional
@@ -139,38 +160,41 @@
   assert(llvm::sys::path::is_absolute(Request.FileName) &&
  "path must be absolute");
 
+  bool ShouldBroadcast = false;
   CDBLookupResult Result;
-  bool SentBroadcast = false;
 
   {
 std::lock_guard Lock(Mutex);
+CachedCDB *Entry;
 if (CompileCommandsDir) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(*CompileCommandsDir);
-  Result.PI.SourceRoot = *CompileCommandsDir;
+  Entry = &getCDBInDirLocked(*CompileCommandsDir);
 } else {
   // Traverse the canonical version to prevent false positives. i.e.:
   // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
+  // FIXME(sammccall): this loop is hot, use a union-find-like structure.
   actOnAllParentDirectories(removeDots(Request.FileName),
-[this, &SentBroadcast, &Result](PathRef Path) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(Path);
-  Result.PI.SourceRoot = Path;
-  return Result.CDB != nullpt

[PATCH] D65332: [Clangd] Disable ExtractVariable for all types of assignments

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah created this revision.
SureYeaah added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65332

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -371,6 +371,8 @@
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
   [[a = 5]];
+  [[a >>= 5]];
+  [[a *= 5]];
   // Variable DeclRefExpr
   a = [[b]];
   // label statement
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -272,7 +272,7 @@
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
-if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+if (BinOpExpr->isAssignmentOp())
   return false;
   }
   // For function and member function DeclRefs, we look for a parent that is a


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -371,6 +371,8 @@
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
   [[a = 5]];
+  [[a >>= 5]];
+  [[a *= 5]];
   // Variable DeclRefExpr
   a = [[b]];
   // label statement
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -272,7 +272,7 @@
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
-if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+if (BinOpExpr->isAssignmentOp())
   return false;
   }
   // For function and member function DeclRefs, we look for a parent that is a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1948
+def RequiresDesignator : InheritableAttr {
+  let Spellings = [Clang<"requires_designator">, GCC<"designated_init">];
+  let Subjects = SubjectList<[Record]>;

Why does this have a `Clang` spelling in addition to the `GCC` spelling? I 
think it only needs the `GCC` spelling.



Comment at: clang/include/clang/Basic/AttrDocs.td:1454-1455
+initializer syntax ([dcl.init.aggr]p3.1).
+For a struct ``Foo`` with one integer field ``x``, the following declarations
+are valid and invalid when this attribute is applied to the struct's 
definition.
+

Please show the struct declaration itself as part of the code block.



Comment at: clang/include/clang/Basic/AttrDocs.td:1464-1466
+For a struct ``Foo`` with three integer fields ``x``, ``y``, and ``z``, the
+following declarations are valid and invalid when this attribute is applied to
+the struct's definition.

Same here.



Comment at: clang/include/clang/Basic/AttrDocs.td:1484-1485
+requires designated initializers be used rather than positional initializers.
+This attribute additionally has a stronger restriction that declarations must 
be
+brace-initialized (which is why ``Foo foo;`` is considered invalid above. The
+final way this attribute differs from GCC's ``designated_init`` attribute is

Why is it valuable to differ from GCC's behavior in this regard?



Comment at: clang/include/clang/Basic/AttrDocs.td:1487
+final way this attribute differs from GCC's ``designated_init`` attribute is
+that it can be applied to union and class types, as well as struct types.
+  }];

Given that it can be added to class types, I wonder what the behavior should be 
for code like:
```
struct Foo {
  int a = 1;
  int b, c;
  int d = 4;
};

Foo foo = {...};
```
Does the initializer for `foo` have to specify `.a` and `.d`?



Comment at: clang/include/clang/Basic/AttrDocs.td:1498
+A field marked with this attribute may not be omitted or default-constructed.
+For a struct ``Foo`` with a ``designated_init_required`` integer field ``x``,
+the following declarations are valid and invalid.

Attribute name seems stale here.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1069
+
+// Warnings for requires_designator and requires_init attributes
+def DesignatedInit : DiagGroup<"designated-init">;

Missing a full stop at the end of the comment.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3537
+def note_declared_requires_designator_here : Note<
+  "required by 'requires_designator' attribute here">;
+def warn_requires_init_failed : Warning<

This attribute spelling seems wrong, it should be `designated_init`, no?



Comment at: clang/lib/Sema/SemaDecl.cpp:11214
 
+  if (const auto *TD = VDecl->getType().getTypePtr()->getAsTagDecl()) {
+// If the type of the declaration is a struct/class and that type has the

You can drop the call to `getTypePtr()` and just use the overloaded 
`operator->()` instead.



Comment at: clang/lib/Sema/SemaDecl.cpp:11216
+// If the type of the declaration is a struct/class and that type has the
+// require_designated_init attribute, check that the initializer has
+// the proper designated initializer syntax.

Attribute spelling is stale in this comment.



Comment at: clang/lib/Sema/SemaDecl.cpp:15314
 
+  // If this TagDecl has any non-public fields, all requires_designator and
+  // requires_init attributes should be ignored.

Attribute name is stale in this comment.

This is the wrong place to do this work -- it should be done from 
SemaDeclAttr.cpp when processing the attribute. We should avoid adding the 
attribute in the first place rather than adding the attribute and then later 
removing it.



Comment at: clang/lib/Sema/SemaDecl.cpp:15318-15321
+for (auto const *FD : RD->fields()) {
+  if (FD->getAccess() != AS_public)
+AllMembersPublic = false;
+}

`HasNonPublicMember = llvm::any_of(RD->fields(), [](const FieldDecl *FD) { 
return FD->getAccess() != AS_public; });`



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5313
+static void handleRequiresInitAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (auto *FD = dyn_cast(D)) {
+auto const *RD = FD->getParent();

No need for this, you can just use `cast<>` directly, as the subject was 
already checked by the common attribute handling code.

I would probably rewrite this as:
```
if (cast(D)->getParent()->isUnion()) {
  S.Diag(...);
  return;
}

D->addAttr(...);
```



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5314
+  if (

[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:168
 std::lock_guard Lock(Mutex);
+CachedCDB *Entry;
 if (CompileCommandsDir) {

nit: set default `nullptr`.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:210
   if (CompileCommandsDir) {
 assert(*CompileCommandsDir == Result.PI.SourceRoot &&
"Trying to broadcast a CDB outside of CompileCommandsDir!");

should we use the `pathEqual` here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65320



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

[PATCH] D65333: [clangd] Refactor computation of extracted expr in ExtractVariable tweak. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: SureYeaah.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This takes this logic out of the Tweak class, and simplifies the signature of
the function where the main logic is.

The goal is to make it easier to turn into a loop like:

  for (current = N; current and current->parent are both expr; current = 
current->parent)
if (suitable(current))
  return current;
  return null;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65333

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -179,54 +179,6 @@
   return tooling::Replacement(SM, InsertionLoc, 0, ExtractedVarDecl);
 }
 
-/// Extracts an expression to the variable dummy
-/// Before:
-/// int x = 5 + 4 * 3;
-/// ^
-/// After:
-/// auto dummy = 5 + 4;
-/// int x = dummy * 3;
-class ExtractVariable : public Tweak {
-public:
-  const char *id() const override final;
-  bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
-  std::string title() const override {
-return "Extract subexpression to variable";
-  }
-  Intent intent() const override { return Refactor; }
-  // Compute the extraction context for the Selection
-  bool computeExtractionContext(const SelectionTree::Node *N,
-const SourceManager &SM, const ASTContext &Ctx);
-
-private:
-  // the expression to extract
-  std::unique_ptr Target;
-};
-REGISTER_TWEAK(ExtractVariable)
-bool ExtractVariable::prepare(const Selection &Inputs) {
-  // we don't trigger on empty selections for now
-  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
-return false;
-  const ASTContext &Ctx = Inputs.AST.getASTContext();
-  const SourceManager &SM = Inputs.AST.getSourceManager();
-  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
-  return computeExtractionContext(N, SM, Ctx);
-}
-
-Expected ExtractVariable::apply(const Selection &Inputs) {
-  tooling::Replacements Result;
-  // FIXME: get variable name from user or suggest based on type
-  std::string VarName = "dummy";
-  // insert new variable declaration
-  if (auto Err = Result.add(Target->insertDeclaration(VarName)))
-return std::move(Err);
-  // replace expression with variable name
-  if (auto Err = Result.add(Target->replaceWithVar(VarName)))
-return std::move(Err);
-  return Effect::applyEdit(Result);
-}
-
 // Find the CallExpr whose callee is an ancestor of the DeclRef
 const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
   // we maintain a stack of all exprs encountered while traversing the
@@ -256,24 +208,22 @@
   return true;
 }
 
-// Find the node that will form our ExtractionContext.
+// Find the Expr node that we're going to extract.
 // We don't want to trigger for assignment expressions and variable/field
 // DeclRefs. For function/member function, we want to extract the entire
 // function call.
-bool ExtractVariable::computeExtractionContext(const SelectionTree::Node *N,
-   const SourceManager &SM,
-   const ASTContext &Ctx) {
+const SelectionTree::Node *computeExtractedExpr(const SelectionTree::Node *N) {
   if (!N)
-return false;
+return nullptr;
   const clang::Expr *SelectedExpr = N->ASTNode.get();
   const SelectionTree::Node *TargetNode = N;
   if (!SelectedExpr)
-return false;
+return nullptr;
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
 if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
-  return false;
+  return nullptr;
   }
   // For function and member function DeclRefs, we look for a parent that is a
   // CallExpr
@@ -281,19 +231,65 @@
   dyn_cast_or_null(SelectedExpr)) {
 // Extracting just a variable isn't that useful.
 if (!isa(DeclRef->getDecl()))
-  return false;
+  return nullptr;
 TargetNode = getCallExpr(N);
   }
   if (const MemberExpr *Member = dyn_cast_or_null(SelectedExpr)) {
 // Extracting just a field member isn't that useful.
 if (!isa(Member->getMemberDecl()))
-  return false;
+  return nullptr;
 TargetNode = getCallExpr(N);
   }
   if (!TargetNode || !canBeAssigned(TargetNode))
+return nullptr;
+  return TargetNode;
+}
+
+/// Extracts an expression to the variable dummy
+/// Before:
+/// int x = 5 + 4 * 3;
+/// ^
+/// After:
+/// auto dummy = 5 + 4;
+/// int x = dummy * 3;
+class ExtractVariable : public Tweak {
+public

[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 4 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:168
 std::lock_guard Lock(Mutex);
+CachedCDB *Entry;
 if (CompileCommandsDir) {

hokein wrote:
> nit: set default `nullptr`.
Done, and also checked for it.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:210
   if (CompileCommandsDir) {
 assert(*CompileCommandsDir == Result.PI.SourceRoot &&
"Trying to broadcast a CDB outside of CompileCommandsDir!");

hokein wrote:
> should we use the `pathEqual` here too?
no, if we have a CompileCommandsDir then we'll only search in that exact 
directory name, and so the result should match exactly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65320



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

[PATCH] D63276: [AST] Add FunctionDecl::getParametersSourceRange()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Decl.h:1928
+  /// macro. Otherwise, it returns the location of the end of the ellipsis.
+  SourceRange getEllipsisSourceRange() const {
+const auto *FPT = getType()->getAs();

Why a source range? The ellipsis is a single token, so a range should be 
unnecessary. I would just have this return `FPT->getEllipsisLoc()`.



Comment at: clang/include/clang/AST/Decl.h:2353
+  /// Attempt to compute an informative source range covering the
+  /// function parameters. The source range is invalid if there are no
+  /// parameters.

Can you add "excluding the parentheses" to this as well, to make it crystal 
clear what is covered?



Comment at: clang/lib/AST/Decl.cpp:3307
+  unsigned NP = getNumParams();
+  auto EllipsisRange = getEllipsisSourceRange();
+

Don't use `auto` unless the type is explicitly spelled out in the 
initialization (elsewhere as well).


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

https://reviews.llvm.org/D63276



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

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

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

baloghadamsoftware updated this revision to Diff 211933.
baloghadamsoftware marked 3 inline comments as done.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D64736

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tidy/bugprone/InfiniteLoopCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-infinite-loop.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-infinite-loop.cpp

Index: test/clang-tidy/bugprone-infinite-loop.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-infinite-loop.cpp
@@ -0,0 +1,322 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t
+
+void simple_infinite_loop1() {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  } while (i < 10);
+
+  for (i = 0; i < 10; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_infinite_loop2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_not_infinite1() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) {
+// Not an error since 'Limit' is updated.
+Limit--;
+  }
+  do {
+Limit--;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; Limit--) {
+  }
+}
+
+void simple_not_infinite2() {
+  for (int i = 10; i-- > 0;) {
+// Not an error, since loop variable is modified in its condition part.
+  }
+}
+
+int unknown_function();
+
+void function_call() {
+  int i = 0;
+  while (i < unknown_function()) {
+// Not an error, since the function may return different values.
+  }
+
+  do {
+// Not an error, since the function may return different values.
+  } while (i < unknown_function());
+
+  for (i = 0; i < unknown_function();) {
+// Not an error, since the function may return different values.
+  }
+}
+
+void escape_before1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) {
+// Not an error, since *p is alias of i.
+(*p)++;
+  }
+
+  do {
+(*p)++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++(*p)) {
+  }
+}
+
+void escape_before2() {
+  int i = 0;
+  int Limit = 100;
+  int &ii = i;
+  while (i < Limit) {
+// Not an error, since ii is alias of i.
+ii++;
+  }
+
+  do {
+ii++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++ii) {
+  }
+}
+
+void escape_inside1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) {
+// Not an error, since *p is alias of i.
+int *p = &i;
+(*p)++;
+  }
+
+  do {
+int *p = &i;
+(*p)++;
+  } while (i < Limit);
+}
+
+void escape_inside2() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) {
+// Not an error, since ii is alias of i.
+int &ii = i;
+ii++;
+  }
+
+  do {
+int &ii = i;
+ii++;
+  } while (i < Limit);
+}
+
+void escape_after1() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+  int *p = &i;
+}
+
+void escape_after2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+  int &ii = i;
+}
+
+int glob;
+
+void global1(int &x) {
+  int i = 0, Limit = 100;
+  while (x < Limit) {
+// Not an error since 'x' can be an alias of 'glob'.
+glob++;
+  }
+}
+
+void global2() {
+  int i = 0, Limit = 100;
+  while (glob < Limit) {
+// Since 'glob' is declared out of the function we do not warn.
+i++;
+  }
+}
+
+struct X {
+  int m;
+
+  void change_m();

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

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

baloghadamsoftware marked 15 inline comments as done.
baloghadamsoftware added a comment.

Thank you for the very throughout review. I updated my patch according to your 
comments and tried to answer your concerns.




Comment at: test/clang-tidy/bugprone-infinite-loop.cpp:156
+  }
+  int &ii = i;
+}

gribozavr wrote:
> Is all escaping that syntactically follows the loop actually irrelevant? For 
> example:
> 
> ```
> int i = 0;
> int j = 0;
> int *p = &j;
> for (int k = 0; k < 5; k++) {
>   *p = 100;
>   if (k != 0) {
> // This loop would be reported as infinite.
> while (i < 10) {
> }
>   }
>   p = &i;
> }
> ```
You are right, but how frequent are such cases? I found no false positives at 
all when checking some ope-source projects. Should we spend resources to detect 
escaping in a nesting loop? I could not find a cheap way yet. I suppose this 
can only be done when nesting two loops. (I am sure we can ignore the cases 
where the outer loop is implemented using a `goto`.



Comment at: test/clang-tidy/bugprone-infinite-loop.cpp:225
+  while (1)
+; //FIXME: We expect report in this case.
+}

gribozavr wrote:
> Why?
> 
> Intentional infinite loops *that perform side-effects in their body* are 
> quite common -- for example, network servers, command-line interpreters etc.
> 
> Also, if the loop body calls an arbitrary function, we don't know if it will 
> throw, or `exit()`, or kill the current thread or whatnot.
We already handle loop exit statements including calls to `[[noreturn]]` 
functions. Is that not enough?


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

https://reviews.llvm.org/D64736



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

[clang-tools-extra] r367112 - [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jul 26 07:07:11 2019
New Revision: 367112

URL: http://llvm.org/viewvc/llvm-project?rev=367112&view=rev
Log:
[clangd] Fix background index not triggering on windows due to case mismatch.

Summary:
This isn't a general fix to all paths where we assume case-sensitivity, it's
a minimally-invasive fix targeting the llvm 9 branch.

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=367112&r1=367111&r2=367112&view=diff
==
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Fri Jul 26 
07:07:11 2019
@@ -117,20 +117,41 @@ DirectoryBasedGlobalCompilationDatabase:
   return None;
 }
 
-std::pair
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+//This class is not the only place this problem exists.
+// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
+
+static std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return Path;
+#endif
+}
+
+static bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
+
+DirectoryBasedGlobalCompilationDatabase::CachedCDB &
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
-  auto CachedIt = CompilationDatabases.find(Dir);
-  if (CachedIt != CompilationDatabases.end())
-return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
-  std::string Error = "";
-
-  CachedCDB Entry;
-  Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
-  auto Result = Entry.CDB.get();
-  CompilationDatabases[Dir] = std::move(Entry);
-
-  return {Result, false};
+  // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
+  auto Key = maybeCaseFoldPath(Dir);
+  auto R = CompilationDatabases.try_emplace(Key);
+  if (R.second) { // Cache miss, try to load CDB.
+CachedCDB &Entry = R.first->second;
+std::string Error = "";
+Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+Entry.Path = Dir;
+  }
+  return R.first->second;
 }
 
 llvm::Optional
@@ -139,38 +160,41 @@ DirectoryBasedGlobalCompilationDatabase:
   assert(llvm::sys::path::is_absolute(Request.FileName) &&
  "path must be absolute");
 
+  bool ShouldBroadcast = false;
   CDBLookupResult Result;
-  bool SentBroadcast = false;
 
   {
 std::lock_guard Lock(Mutex);
+CachedCDB *Entry = nullptr;
 if (CompileCommandsDir) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(*CompileCommandsDir);
-  Result.PI.SourceRoot = *CompileCommandsDir;
+  Entry = &getCDBInDirLocked(*CompileCommandsDir);
 } else {
   // Traverse the canonical version to prevent false positives. i.e.:
   // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
+  // FIXME(sammccall): this loop is hot, use a union-find-like structure.
   actOnAllParentDirectories(removeDots(Request.FileName),
-[this, &SentBroadcast, &Result](PathRef Path) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(Path);
-  Result.PI.SourceRoot = Path;
-  return Result.CDB != nullptr;
+[&](PathRef Path) {
+  Entry = &getCDBInDirLocked(Path);
+  return Entry->CDB != nullptr;
 });
 }
 
-if (!Result.CDB)
+if (!Entry || !Entry->CDB)
   return llvm::None;
 
 // Mark CDB as broadcasted to make sure discovery is performed once.
-if (Request.ShouldBroadcast && !SentBroadcast)
-  CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
+if (Request.ShouldBroadcast && !Entry->SentBroadcast) {
+  Entry->SentBroadcast = true;
+  ShouldBroadcast = true;
+}
+
+Result.CDB = Entry->CDB.get();
+Result.PI.SourceRoot = Entry->Path;
   }
 
   // FIXME: Maybe make the following part async, since this can block retrieval
   // of compile commands.
-  

[clang-tools-extra] r367113 - [Clangd] Disable ExtractVariable for all types of assignments

[Shaurya Gupta via cfe-commits]

Author: sureyeaah
Date: Fri Jul 26 07:08:27 2019
New Revision: 367113

URL: http://llvm.org/viewvc/llvm-project?rev=367113&view=rev
Log:
[Clangd] Disable ExtractVariable for all types of assignments

Reviewers: sammccall, kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp?rev=367113&r1=367112&r2=367113&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp Fri Jul 
26 07:08:27 2019
@@ -272,7 +272,7 @@ bool ExtractVariable::computeExtractionC
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
-if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+if (BinOpExpr->isAssignmentOp())
   return false;
   }
   // For function and member function DeclRefs, we look for a parent that is a

Modified: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp?rev=367113&r1=367112&r2=367113&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp Fri Jul 26 07:08:27 
2019
@@ -371,6 +371,8 @@ TEST(TweakTest, ExtractVariable) {
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
   [[a = 5]];
+  [[a >>= 5]];
+  [[a *= 5]];
   // Variable DeclRefExpr
   a = [[b]];
   // label statement


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

[PATCH] D65320: [clangd] Fix background index not triggering on windows due to case mismatch.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rL367112: [clangd] Fix background index not triggering on 
windows due to case mismatch. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65320?vs=211921&id=211936#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65320

Files:
  clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h

Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
@@ -117,20 +117,41 @@
   return None;
 }
 
-std::pair
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+//This class is not the only place this problem exists.
+// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
+
+static std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return Path;
+#endif
+}
+
+static bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
+
+DirectoryBasedGlobalCompilationDatabase::CachedCDB &
 DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
   // FIXME(ibiryukov): Invalidate cached compilation databases on changes
-  auto CachedIt = CompilationDatabases.find(Dir);
-  if (CachedIt != CompilationDatabases.end())
-return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
-  std::string Error = "";
-
-  CachedCDB Entry;
-  Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
-  auto Result = Entry.CDB.get();
-  CompilationDatabases[Dir] = std::move(Entry);
-
-  return {Result, false};
+  // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
+  auto Key = maybeCaseFoldPath(Dir);
+  auto R = CompilationDatabases.try_emplace(Key);
+  if (R.second) { // Cache miss, try to load CDB.
+CachedCDB &Entry = R.first->second;
+std::string Error = "";
+Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+Entry.Path = Dir;
+  }
+  return R.first->second;
 }
 
 llvm::Optional
@@ -139,38 +160,41 @@
   assert(llvm::sys::path::is_absolute(Request.FileName) &&
  "path must be absolute");
 
+  bool ShouldBroadcast = false;
   CDBLookupResult Result;
-  bool SentBroadcast = false;
 
   {
 std::lock_guard Lock(Mutex);
+CachedCDB *Entry = nullptr;
 if (CompileCommandsDir) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(*CompileCommandsDir);
-  Result.PI.SourceRoot = *CompileCommandsDir;
+  Entry = &getCDBInDirLocked(*CompileCommandsDir);
 } else {
   // Traverse the canonical version to prevent false positives. i.e.:
   // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
+  // FIXME(sammccall): this loop is hot, use a union-find-like structure.
   actOnAllParentDirectories(removeDots(Request.FileName),
-[this, &SentBroadcast, &Result](PathRef Path) {
-  std::tie(Result.CDB, SentBroadcast) =
-  getCDBInDirLocked(Path);
-  Result.PI.SourceRoot = Path;
-  return Result.CDB != nullptr;
+[&](PathRef Path) {
+  Entry = &getCDBInDirLocked(Path);
+  return Entry->CDB != nullptr;
 });
 }
 
-if (!Result.CDB)
+if (!Entry || !Entry->CDB)
   return llvm::None;
 
 // Mark CDB as broadcasted to make sure discovery is performed once.
-if (Request.ShouldBroadcast && !SentBroadcast)
-  CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
+if (Request.ShouldBroadcast && !Entry->SentBroadcast) {
+  Entry->SentBroadcast = true;
+  ShouldBroadcast = true;
+}
+
+Result.CDB = Entry->CDB.get();
+Result.PI.SourceRoot = Entry->Path;
   }
 
   // FIXME: Maybe make the following part async, since this can block retrieval
   // of compile commands.
-  if (Request.ShouldBroadcast && !SentBroadcast)
+  if (ShouldBroadcast)
 broadcastCDB(Result);
   return Result;
 }
@@ -200,9 +224,9 @@
 if (!It.second)
   return true;
 
-auto Res = getCDBInDirLocked(Path);
-It.f

[PATCH] D65332: [Clangd] Disable ExtractVariable for all types of assignments

[Shaurya Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367113: [Clangd] Disable ExtractVariable for all types of 
assignments (authored by SureYeaah, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65332?vs=211927&id=211937#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65332

Files:
  clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -272,7 +272,7 @@
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
-if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+if (BinOpExpr->isAssignmentOp())
   return false;
   }
   // For function and member function DeclRefs, we look for a parent that is a
Index: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
@@ -371,6 +371,8 @@
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
   [[a = 5]];
+  [[a >>= 5]];
+  [[a *= 5]];
   // Variable DeclRefExpr
   a = [[b]];
   // label statement


Index: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -272,7 +272,7 @@
   // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
   if (const BinaryOperator *BinOpExpr =
   dyn_cast_or_null(SelectedExpr)) {
-if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+if (BinOpExpr->isAssignmentOp())
   return false;
   }
   // For function and member function DeclRefs, we look for a parent that is a
Index: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
@@ -371,6 +371,8 @@
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
   [[a = 5]];
+  [[a >>= 5]];
+  [[a *= 5]];
   // Variable DeclRefExpr
   a = [[b]];
   // label statement
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r367116 - [OPENMP]Add support for analysis of reduction variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jul 26 07:50:05 2019
New Revision: 367116

URL: http://llvm.org/viewvc/llvm-project?rev=367116&view=rev
Log:
[OPENMP]Add support for analysis of reduction variables.

Summary:
Reduction variables are the variables, for which the private copies
must be created in the OpenMP regions. Then they are initialized with
the predefined values depending on the reduction operation. After exit
from the OpenMP region the original variable is updated using the
reduction value and the value of the original reduction variable.

Reviewers: NoQ

Subscribers: guansong, jdoerfert, caomhin, kkwli0, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/test/Analysis/cfg-openmp.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_reduction_messages.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/distribute_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/for_reduction_messages.cpp
cfe/trunk/test/OpenMP/for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/parallel_for_reduction_messages.cpp
cfe/trunk/test/OpenMP/parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/parallel_reduction_messages.cpp
cfe/trunk/test/OpenMP/parallel_sections_reduction_messages.cpp
cfe/trunk/test/OpenMP/sections_reduction_messages.cpp
cfe/trunk/test/OpenMP/simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_simd_reduction_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_reduction_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_reduction_messages.cpp
cfe/trunk/test/OpenMP/taskloop_reduction_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_reduction_messages.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/teams_reduction_messages.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=367116&r1=367115&r2=367116&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Fri Jul 26 07:50:05 2019
@@ -2618,10 +2618,12 @@ public:
   }
 
   child_range used_children() {
-return child_range(child_iterator(), child_iterator());
+return child_range(reinterpret_cast(varlist_begin()),
+   reinterpret_cast(varlist_end()));
   }
   const_child_range used_children() const {
-return const_child_range(const_child_iterator(), const_child_iterator());
+auto Children = const_cast(this)->used_children();
+return const_child_range(Children.begin(), Children.end());
   }
 
   static bool classof(const OMPClause *T) {

Modified: cfe/trunk/test/Analysis/cfg-openmp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-openmp.cpp?rev=367116&r1=367115&r2=367116&view=diff
==
--- cfe/trunk/test/Analysis/cfg-openmp.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-openmp.cpp Fri Jul 26 07:50:05 2019
@@ -6,7 +6,8 @@ void xxx(int argc) {
 // CHECK-NEXT:   1: int x;
 // CHECK-NEXT:   2: int cond;
 // CHECK-NEXT:   3: int fp;
-  int x, cond, fp;
+// CHECK-NEXT:   4: int rd;
+  int x, cond, fp, rd;
 // CHECK-NEXT:   [[#ATOM:]]: x
 // CHECK-NEXT:   [[#ATOM+1]]: [B1.[[#ATOM]]] (ImplicitCastExpr, 
LValueToRValue, int)
 // CHECK-NEXT:   [[#ATOM+2]]: argc
@@ -31,10 +32,11 @@ void xxx(int argc) {
 // CHECK-NEXT:  [[#DPF+5]]: [B1.[[#DPF+4]]] (ImplicitCastExpr, LValueToRValue, 
int)
 // CHECK-NEXT:  [[#DPF+6]]: [B1.[[#DPF+5]]] (ImplicitCastExpr, 
IntegralToBoolean, _Bool)
 // CHECK-NEXT:  [[#DPF+7]]: fp
-// CHECK-NEXT:  [[#DPF+8]]: #pragma omp distribute parallel for if(parallel: 
cond) firstprivate(fp)
+// CHECK-NEXT:  [[#DPF+8]]: rd
+// CHECK-NEXT:  [[#DPF+9]]: #pragma omp distribute parallel for if(parallel: 
cond) firstprivate(fp) reduction(+: rd)
 // CHECK-NEXT:for (int i = 0; i < 10; ++i)
 // CHECK-NEXT:[B1.[[#DPF+3]]];
-#pragma omp distribute parallel for if(parallel:cond) first

r367119 - [ARM] Set default alignment to 64bits

[Simi Pallipurath via cfe-commits]

Author: simpal01
Date: Fri Jul 26 08:05:19 2019
New Revision: 367119

URL: http://llvm.org/viewvc/llvm-project?rev=367119&view=rev
Log:
[ARM] Set default alignment to 64bits

The maximum alignment used by ARM arch
is 64bits, not 128.

This could cause overaligned memory
access for 128 bit neon vector that
have unpredictable behaviour.

This fixes: https://bugs.llvm.org/show_bug.cgi?id=42668

Patch by: Diogo Sampaio(diogo.samp...@arm.com)

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

Change-Id: I5a62b766491f15dd51e4cfe6625929db897f67e3

Added:
cfe/trunk/test/CodeGen/ARM/
cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp

Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=367119&r1=367118&r2=367119&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Fri Jul 26 08:05:19 2019
@@ -309,8 +309,9 @@ ARMTargetInfo::ARMTargetInfo(const llvm:
   setAtomic();
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
+  // as well the default alignment
   if (IsAAPCS && (Triple.getEnvironment() != llvm::Triple::Android))
-MaxVectorAlign = 64;
+DefaultAlignForAttributeAligned = MaxVectorAlign = 64;
 
   // Do force alignment of members that follow zero length bitfields.  If
   // the alignment of the zero-length bitfield is greater than the member

Added: cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp?rev=367119&view=auto
==
--- cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp (added)
+++ cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp Fri Jul 26 08:05:19 2019
@@ -0,0 +1,19 @@
+// Bug: https://bugs.llvm.org/show_bug.cgi?id=42668
+// REQUIRES: arm-registered-target
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8-a -S -emit-llvm -Os -o 
- %s | FileCheck --check-prefixes=CHECK,A8 %s
+// RUN: %clang --target=arm-linux-androideabi -march=armv8-a -S -emit-llvm -Os 
-o - %s | FileCheck --check-prefixes=CHECK,A16 %s
+// CHECK: [[E:%[A-z0-9]+]] = tail call i8* @__cxa_allocate_exception
+// CHECK-NEXT: [[BC:%[A-z0-9]+]] = bitcast i8* [[E]] to <2 x i64>*
+// A8-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 8
+// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16
+#include 
+
+int main(void) {
+  try {
+throw vld1q_u64(((const uint64_t[2]){1, 2}));
+  } catch (uint64x2_t exc) {
+return 0;
+  }
+  return 1;
+}
+

Modified: cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp?rev=367119&r1=367118&r2=367119&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp Fri Jul 26 08:05:19 
2019
@@ -2,11 +2,12 @@
 // RUN: %clang_cc1 -triple arm64-apple-ios10 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos10 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos4 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions  -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-ios12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos5 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
-// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm-linux-androideabi -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple aarch64-linux-gnueabi -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple mipsel-linux-gnu -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple mips64el-linux-gnu -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s


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

[PATCH] D65043: [Format] Add C++20 standard to style options

[Brian Gesiak via Phabricator via cfe-commits]

modocache marked 2 inline comments as done.
modocache added inline comments.



Comment at: clang/lib/Format/Format.cpp:2373
   LangOpts.CPlusPlus17 = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
-  LangOpts.CPlusPlus2a = Style.Standard == FormatStyle::LS_Cpp03 ? 0 : 1;
+  LangOpts.CPlusPlus2a = Style.Standard == FormatStyle::LS_Cpp20 ? 1 : 0;
   LangOpts.LineComment = 1;

Quuxplusone wrote:
> Incidentally, these four lines seem like a great place to use `Style.Standard 
> >= FormatStyle::LS_CppWhatever` (with a cast if necessary), unless that's 
> against some style rule.
This has already been updated to do so in D65183.



Comment at: clang/unittests/Format/FormatTest.cpp:13815
+  verifyFormat("co_yield++ i;");
+  verifyFormat("co_yield ++i;", Cpp20);
+

Quuxplusone wrote:
> If you're going to test C++11's behavior here, please use `co_yield - 1;` or 
> something else that might reasonably appear in a C++11 program. `co_yield++ 
> i;` is not valid C++ (unless `i` is a macro, I guess).
`i` would be an iterator in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65043



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

[PATCH] D65149: [Format] Add test demonstrating PR42722

[Brian Gesiak via Phabricator via cfe-commits]

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

Sure thing! Just to be clear: this test doesn't fail, it passes. My intention 
was to commit this, then commit a patch that improved the indentation behavior, 
which would also include a change to the test that demonstrated the new 
behavior. But, as per your suggestion, I'll wait for the fix before committing 
this. Thanks!




Comment at: clang/unittests/Format/FormatTest.cpp:5184
+   "c++;\n"
+   "d++\n"
+   "  });\n"

This is a passing test that demonstrates that the indentation of `c++` and 
`d++` here is wacky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65149



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

[PATCH] D65106: [OPENMP]Add support for analysis of reduction variables.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367116: [OPENMP]Add support for analysis of reduction 
variables. (authored by ABataev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65106

Files:
  cfe/trunk/include/clang/AST/OpenMPClause.h
  cfe/trunk/test/Analysis/cfg-openmp.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_reduction_messages.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/distribute_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/for_reduction_messages.cpp
  cfe/trunk/test/OpenMP/for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/parallel_for_reduction_messages.cpp
  cfe/trunk/test/OpenMP/parallel_for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/parallel_reduction_messages.cpp
  cfe/trunk/test/OpenMP/parallel_sections_reduction_messages.cpp
  cfe/trunk/test/OpenMP/sections_reduction_messages.cpp
  cfe/trunk/test/OpenMP/simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_parallel_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_simd_reduction_messages.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_reduction_messages.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/target_teams_reduction_messages.cpp
  cfe/trunk/test/OpenMP/taskloop_reduction_messages.cpp
  cfe/trunk/test/OpenMP/taskloop_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/teams_distribute_parallel_for_reduction_messages.cpp
  
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
  cfe/trunk/test/OpenMP/teams_distribute_simd_reduction_messages.cpp
  cfe/trunk/test/OpenMP/teams_reduction_messages.cpp

Index: cfe/trunk/include/clang/AST/OpenMPClause.h
===
--- cfe/trunk/include/clang/AST/OpenMPClause.h
+++ cfe/trunk/include/clang/AST/OpenMPClause.h
@@ -2618,10 +2618,12 @@
   }
 
   child_range used_children() {
-return child_range(child_iterator(), child_iterator());
+return child_range(reinterpret_cast(varlist_begin()),
+   reinterpret_cast(varlist_end()));
   }
   const_child_range used_children() const {
-return const_child_range(const_child_iterator(), const_child_iterator());
+auto Children = const_cast(this)->used_children();
+return const_child_range(Children.begin(), Children.end());
   }
 
   static bool classof(const OMPClause *T) {
Index: cfe/trunk/test/OpenMP/sections_reduction_messages.cpp
===
--- cfe/trunk/test/OpenMP/sections_reduction_messages.cpp
+++ cfe/trunk/test/OpenMP/sections_reduction_messages.cpp
@@ -7,6 +7,16 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -std=c++11 -ferror-limit 150 -o - %s -Wuninitialized
 
 extern int omp_default_mem_alloc;
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp parallel
+#pragma omp sections reduction(+:fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  {
+for (int i = 0; i < 10; ++i)
+  ;
+  }
+}
+
 void foo() {
 }
 
Index: cfe/trunk/test/OpenMP/distribute_parallel_for_simd_reduction_messages.cpp
===
--- cfe/trunk/test/OpenMP/distribute_parallel_for_simd_reduction_messages.cpp
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_simd_reduction_messages.cpp
@@ -7,6 +7,13 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -std=c++11 -ferror-limit 150 -o - %s -Wno-openmp-target -Wuninitialized
 
 extern int omp_default_mem_alloc;
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp distribute parallel for simd reduction(+:fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  for (int i = 0; i < 10; ++i)
+;
+}
+
 void foo() {
 }
 
Index: cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
===
--- cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
+++ cfe/trunk/test/OpenMP/teams_distribute_reduction_messages.cpp
@@ -7,6 +7,14 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -std=c++11 %s -Wno-openmp-target -Wuninitialized
 
 extern int omp_default_mem_alloc;
+void xxx(int argc) {
+  int fp; /

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

`ln -n` is not a standard option that is portably supported. I'll be restoring 
the use of `rm` (with `-f` instead of `-rf`) and the associated comment for 
removing `%t.fake`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64301



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

[PATCH] D65000: [ARM] Set default alignment to 64bits

[Simi Pallipurath via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367119: [ARM] Set default alignment to 64bits (authored by 
simpal01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65000

Files:
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
  cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp


Index: cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
===
--- cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
+++ cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
@@ -2,11 +2,12 @@
 // RUN: %clang_cc1 -triple arm64-apple-ios10 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos10 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos4 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions  -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-ios12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos12 -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos5 -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
-// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm-linux-androideabi -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple aarch64-linux-gnueabi -verify -fsyntax-only 
-std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple mipsel-linux-gnu -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple mips64el-linux-gnu -verify -fsyntax-only -std=c++11 
-fcxx-exceptions -fexceptions %s
Index: cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
===
--- cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
+++ cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp
@@ -0,0 +1,19 @@
+// Bug: https://bugs.llvm.org/show_bug.cgi?id=42668
+// REQUIRES: arm-registered-target
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8-a -S -emit-llvm -Os -o 
- %s | FileCheck --check-prefixes=CHECK,A8 %s
+// RUN: %clang --target=arm-linux-androideabi -march=armv8-a -S -emit-llvm -Os 
-o - %s | FileCheck --check-prefixes=CHECK,A16 %s
+// CHECK: [[E:%[A-z0-9]+]] = tail call i8* @__cxa_allocate_exception
+// CHECK-NEXT: [[BC:%[A-z0-9]+]] = bitcast i8* [[E]] to <2 x i64>*
+// A8-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 8
+// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16
+#include 
+
+int main(void) {
+  try {
+throw vld1q_u64(((const uint64_t[2]){1, 2}));
+  } catch (uint64x2_t exc) {
+return 0;
+  }
+  return 1;
+}
+
Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -309,8 +309,9 @@
   setAtomic();
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
+  // as well the default alignment
   if (IsAAPCS && (Triple.getEnvironment() != llvm::Triple::Android))
-MaxVectorAlign = 64;
+DefaultAlignForAttributeAligned = MaxVectorAlign = 64;
 
   // Do force alignment of members that follow zero length bitfields.  If
   // the alignment of the zero-length bitfield is greater than the member


Index: cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
===
--- cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
+++ cfe/trunk/test/SemaCXX/warn-overaligned-type-thrown.cpp
@@ -2,11 +2,12 @@
 // RUN: %clang_cc1 -triple arm64-apple-ios10 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-tvos10 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple arm64-apple-watchos4 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions  -DUNDERALIGNED %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
 // RUN: %clang_cc1 -triple arm64-apple-ios12 -verify -fsyntax-only -std=c++11 -fc

[clang-tools-extra] r367121 - [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jul 26 08:29:52 2019
New Revision: 367121

URL: http://llvm.org/viewvc/llvm-project?rev=367121&view=rev
Log:
[clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

Summary:
These aren't formally subexpressions in C++, in this case + is left-associative.
However informally +, *, etc are usually (mathematically) associative and users
consider these subexpressions.

We detect these and in simple cases support extracting the partial expression.
As well as builtin associative operators, we assume that overloads of them
are associative and support those too.

Reviewers: SureYeaah

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Selection.cpp
clang-tools-extra/trunk/clangd/Selection.h
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/Selection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Selection.cpp?rev=367121&r1=367120&r2=367121&view=diff
==
--- clang-tools-extra/trunk/clangd/Selection.cpp (original)
+++ clang-tools-extra/trunk/clangd/Selection.cpp Fri Jul 26 08:29:52 2019
@@ -452,5 +452,12 @@ const DeclContext& SelectionTree::Node::
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
 
+const SelectionTree::Node &SelectionTree::Node::ignoreImplicit() const {
+  if (Children.size() == 1 &&
+  Children.front()->ASTNode.getSourceRange() == ASTNode.getSourceRange())
+return Children.front()->ignoreImplicit();
+  return *this;
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/Selection.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Selection.h?rev=367121&r1=367120&r2=367121&view=diff
==
--- clang-tools-extra/trunk/clangd/Selection.h (original)
+++ clang-tools-extra/trunk/clangd/Selection.h Fri Jul 26 08:29:52 2019
@@ -103,6 +103,9 @@ public:
 const DeclContext& getDeclContext() const;
 // Printable node kind, like "CXXRecordDecl" or "AutoTypeLoc".
 std::string kind() const;
+// If this node is a wrapper with no syntax (e.g. implicit cast), return
+// its contents. (If multiple wrappers are present, unwraps all of them).
+const Node& ignoreImplicit() const;
   };
   // The most specific common ancestor of all the selected nodes.
   // Returns nullptr if the common ancestor is the root.

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp?rev=367121&r1=367120&r2=367121&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp Fri Jul 
26 08:29:52 2019
@@ -27,6 +27,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -39,10 +40,14 @@ public:
   const clang::Expr *getExpr() const { return Expr; }
   const SelectionTree::Node *getExprNode() const { return ExprNode; }
   bool isExtractable() const { return Extractable; }
+  // The half-open range for the expression to be extracted.
+  SourceRange getExtractionChars() const;
   // Generate Replacement for replacing selected expression with given VarName
-  tooling::Replacement replaceWithVar(llvm::StringRef VarName) const;
+  tooling::Replacement replaceWithVar(SourceRange Chars,
+  llvm::StringRef VarName) const;
   // Generate Replacement for declaring the selected Expr as a new variable
-  tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+  tooling::Replacement insertDeclaration(llvm::StringRef VarName,
+ SourceRange InitChars) const;
 
 private:
   bool Extractable = false;
@@ -152,23 +157,20 @@ const clang::Stmt *ExtractionContext::co
   }
   return nullptr;
 }
+
 // returns the replacement for substituting the extraction with VarName
 tooling::Replacement
-ExtractionContext::replaceWithVar(llvm::StringRef VarName) const {
-  const llvm::Optional ExtractionRng =
-  toHalfOpenFileRange(SM, Ctx.getLangOpts(), getExpr()->getSourceRange());
-  unsigned ExtractionLength = SM.getFileOffset(ExtractionRng->getEnd()) -
-  SM.getFileOffset(ExtractionRng->getBegin());
-  return tooling::Replacement(SM, ExtractionRng-

[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367121: [clangd] Support extraction of binary 
"subexpressions" like a + [[b + c]]. (authored by sammccall, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65139?vs=211905&id=211945#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65139

Files:
  clang-tools-extra/trunk/clangd/Selection.cpp
  clang-tools-extra/trunk/clangd/Selection.h
  clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/trunk/clangd/Selection.cpp
===
--- clang-tools-extra/trunk/clangd/Selection.cpp
+++ clang-tools-extra/trunk/clangd/Selection.cpp
@@ -452,5 +452,12 @@
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
 
+const SelectionTree::Node &SelectionTree::Node::ignoreImplicit() const {
+  if (Children.size() == 1 &&
+  Children.front()->ASTNode.getSourceRange() == ASTNode.getSourceRange())
+return Children.front()->ignoreImplicit();
+  return *this;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -27,6 +27,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -39,10 +40,14 @@
   const clang::Expr *getExpr() const { return Expr; }
   const SelectionTree::Node *getExprNode() const { return ExprNode; }
   bool isExtractable() const { return Extractable; }
+  // The half-open range for the expression to be extracted.
+  SourceRange getExtractionChars() const;
   // Generate Replacement for replacing selected expression with given VarName
-  tooling::Replacement replaceWithVar(llvm::StringRef VarName) const;
+  tooling::Replacement replaceWithVar(SourceRange Chars,
+  llvm::StringRef VarName) const;
   // Generate Replacement for declaring the selected Expr as a new variable
-  tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+  tooling::Replacement insertDeclaration(llvm::StringRef VarName,
+ SourceRange InitChars) const;
 
 private:
   bool Extractable = false;
@@ -152,23 +157,20 @@
   }
   return nullptr;
 }
+
 // returns the replacement for substituting the extraction with VarName
 tooling::Replacement
-ExtractionContext::replaceWithVar(llvm::StringRef VarName) const {
-  const llvm::Optional ExtractionRng =
-  toHalfOpenFileRange(SM, Ctx.getLangOpts(), getExpr()->getSourceRange());
-  unsigned ExtractionLength = SM.getFileOffset(ExtractionRng->getEnd()) -
-  SM.getFileOffset(ExtractionRng->getBegin());
-  return tooling::Replacement(SM, ExtractionRng->getBegin(), ExtractionLength,
-  VarName);
+ExtractionContext::replaceWithVar(SourceRange Chars,
+  llvm::StringRef VarName) const {
+  unsigned ExtractionLength =
+  SM.getFileOffset(Chars.getEnd()) - SM.getFileOffset(Chars.getBegin());
+  return tooling::Replacement(SM, Chars.getBegin(), ExtractionLength, VarName);
 }
 // returns the Replacement for declaring a new variable storing the extraction
 tooling::Replacement
-ExtractionContext::insertDeclaration(llvm::StringRef VarName) const {
-  const llvm::Optional ExtractionRng =
-  toHalfOpenFileRange(SM, Ctx.getLangOpts(), getExpr()->getSourceRange());
-  assert(ExtractionRng && "ExtractionRng should not be null");
-  llvm::StringRef ExtractionCode = toSourceCode(SM, *ExtractionRng);
+ExtractionContext::insertDeclaration(llvm::StringRef VarName,
+ SourceRange InitializerChars) const {
+  llvm::StringRef ExtractionCode = toSourceCode(SM, InitializerChars);
   const SourceLocation InsertionLoc =
   toHalfOpenFileRange(SM, Ctx.getLangOpts(),
   InsertionPoint->getSourceRange())
@@ -179,6 +181,144 @@
   return tooling::Replacement(SM, InsertionLoc, 0, ExtractedVarDecl);
 }
 
+// Helpers for handling "binary subexpressions" like a + [[b + c]] + d.
+//
+// These are special, because the formal AST doesn't match what users expect:
+// - the AST is ((a + b) + c) + d, so the ancestor expression is `a + b + c`.
+// - but extracting `b + c` is reasonable, as + is (mathematically) associative.
+//
+// So we try to support these cases with some restrictions:
+//  -

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  It seems reasonable to me to allow OpenCL vector initialization syntax 
in OpenCL C++, since we're treating OpenCL C++ as an ObjC++-like merge of the 
language extensions, but we shouldn't go further than that and start changing 
behavior in the non-OpenCL languages.


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

https://reviews.llvm.org/D65286



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

[PATCH] D65043: [Format] Add C++20 standard to style options

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:13815
+  verifyFormat("co_yield++ i;");
+  verifyFormat("co_yield ++i;", Cpp20);
+

modocache wrote:
> Quuxplusone wrote:
> > If you're going to test C++11's behavior here, please use `co_yield - 1;` 
> > or something else that might reasonably appear in a C++11 program. 
> > `co_yield++ i;` is not valid C++ (unless `i` is a macro, I guess).
> `i` would be an iterator in this case.
No matter what `i` is, `co_yield++ i;` is never a valid sequence of tokens in 
C++17 (unless `i` is a macro, I guess).

`co_yield ++i;` will become valid in C++2a, but if you see that sequence of 
tokens in any C++ program ever, you can just assume that it is C++2a. There is 
never any reason to think that it should be formatted as `co_yield++ i;`, 
because that would not be valid C++17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65043



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

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 211956.
Charusso edited the summary of this revision.
Charusso added a comment.

- Restrict the generic contradiction-based range evaluation to only affect that 
left-hand side operands which constraint range are concrete zero.


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

https://reviews.llvm.org/D65239

Files:
  clang/include/clang/AST/Expr.h
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/bitwise-nullability.cpp
  clang/test/Analysis/bitwise-ranges.cpp

Index: clang/test/Analysis/bitwise-ranges.cpp
===
--- /dev/null
+++ clang/test/Analysis/bitwise-ranges.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s 2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+
+void test_range(unsigned int X) {
+  if (X < 2 || X > 3)
+return;
+
+  unsigned int A = X & 1;
+  unsigned int B = X & 8;
+  unsigned int C = X | 8;
+  unsigned int D = X << 1;
+  unsigned int E = X >> 1;
+
+  if (A && B && C && D && E) {}
+
+  clang_analyzer_printState();
+
+  // CHECK: { "symbol": "reg_$0", "range": "{ [2, 3] }" }
+  // CHECK: { "symbol": "(reg_$0) & 1U", "range": "{ [1, 1] }" }
+  // CHECK: { "symbol": "(reg_$0) & 8U", "range": "{ [1, 8] }" }
+  // CHECK: { "symbol": "(reg_$0) << 1U", "range": "{ [1, 4294967295] }" }
+  // CHECK: { "symbol": "(reg_$0) >> 1U", "range": "{ [1, 4294967295] }" }
+
+  void(X / 0);
+  // expected-warning@-1 {{division by zero is undefined}}
+  // expected-warning@-2 {{Division by zero}}
+}
Index: clang/test/Analysis/bitwise-nullability.cpp
===
--- /dev/null
+++ clang/test/Analysis/bitwise-nullability.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core \
+// RUN:  -analyzer-output=text -verify %s
+
+typedef unsigned long long uint64_t;
+
+void test_null_lhs_range_to_non_null_result_infeasible(uint64_t Magic) {
+  uint64_t MaskedMagic = Magic & (0xULL >> 13);
+
+  if (Magic) {
+// no-note: 'Assuming 'Magic' is 0' was here.
+return;
+  }
+
+  if (MaskedMagic) {
+// no-note: 'Assuming 'MaskedMagic' is not equal to 0' was here.
+(void)(1 / Magic); // no-warning
+  }
+}
+
+void test_non_null_lhs_range_to_null_result_feasible(uint64_t Magic) {
+  uint64_t MaskedMagic = Magic & (0xULL >> 13);
+
+  if (!Magic) {
+// expected-note@-1 {{Assuming 'Magic' is not equal to 0}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (!MaskedMagic) {
+// expected-note@-1 {{Assuming 'MaskedMagic' is 0}}
+// expected-note@-2 {{Taking true branch}}
+(void)(1 / !Magic);
+// expected-note@-1 {{'Magic' is not equal to 0}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -476,6 +476,55 @@
   return Input;
 }
 
+// We do not support evaluating bitwise operations, so that when we check for
+// their results being null we create a new assumption whether the current new
+// symbol is null or non-null. If we are on the non-null assumption's branch
+// we need to check the left-hand side operand's constraint range informations:
+// - It if contradicts with the forming new range 'RS' then it returns a null
+//   state as it is an impossible condition.
+// - Otherwise it removes the nullability from its ranges as we know that it
+//   cannot be null on that branch.
+static ProgramStateRef applyBitwiseRanges(ProgramStateRef State,
+  BasicValueFactory &BV,
+  RangeSet::Factory &F, RangeSet RS,
+  SymbolRef Sym) {
+  if (RS.isEmpty())
+return State;
+
+  // If the 'RS' is zero we cannot apply new range informations at that branch
+  // as a bitwise operation result could be null.
+  if (RS.getConcreteValue() && RS.getConcreteValue()->getExtValue() == 0)
+return State;
+
+  const SymIntExpr *SIE = dyn_cast(Sym);
+  if (!SIE)
+return State;
+
+  if (!BinaryOperator::isBitwiseOrShiftOp(SIE->getOpcode()))
+return State;
+
+  ConstraintRangeTy Constraints = State->get();
+  for (const SymbolRef CurrentSym : SIE->symbols()) {
+if (CurrentSym == SIE)
+  continue;
+
+if (const RangeSet *CurrentRS = Constraints.lookup(CurrentSym)) {
+  const RangeSet NewRS = assu

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 3 inline comments as done.
Charusso added a comment.

In D65239#1599889 , @NoQ wrote:

> Aha, great, the overall structure of the code is correct!


Thanks! Now it is more formal. The only problem it does not change anything on 
LLVM reports, where we are working with the 
`test_non_null_lhs_range_to_null_result_feasible` test case.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:477
+
+  // For all of the bitwise operations,
+  // if they remain in that 'SymIntExpr' form that means we cannot evaluate the

NoQ wrote:
> Let's do some math.
> 
> Suppose `$x` is in range `[2, 3]`. In this case the true range for `$x & 1` 
> is `[0, 1]` (because `2 & 1 == 0` and `3 & 1 == 1`).
> 
> The range for `$x & 8` would be `[0, 0]`.
> 
> The range for `$x | 8` would be `[10, 11]`.
> 
> The range for `$x << 1` would be `[4, 4], [6, 6]`.
> 
> The range for `$x >> 1` would be `[0, 1]`.
> 
> None of these ranges are contained within `[2, 3]`. In fact, none of them 
> even contain either `2` or `3`. However, when you intersect the resulting 
> range with `[2, 3]`, you make sure that the resulting range is contained 
> within `[2, 3]`. I don't think that's correct.
I have added a test case to see how bad our evaluation at the moment. That is 
why I saw that we are just narrowing ranges and the contradiction only could 
happen at concrete zero range based.


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

https://reviews.llvm.org/D65239



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

[PATCH] D65309: [clang-format] Fix style of css file paths

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM after adding a comment to explain




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:234
 llvm::sys::path::filename(FilePath));
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);

Comment here why you're explicitly using posix (i.e. HTML uses posix-style 
paths)


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

https://reviews.llvm.org/D65309



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder marked an inline comment as done.
tmroeder added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:77
+
+  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return

Eugene.Zelenko wrote:
> Release Notes should include short description. One sentence is enough, but 
> it'll good idea to keep it same as first statement of documentation.
Would you like me to submit a patch that removes the second paragraph of this 
description, then?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder marked an inline comment as done.
tmroeder added inline comments.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
+

nickdesaulniers wrote:
> tmroeder wrote:
> > lebedev.ri wrote:
> > > This looks wrong
> > Yeah, I'm not sure where that came from. I'll remove it.
> Hard to tell, but I don't think this `using` statement was ever removed as 
> requested?
It's not the "using" statement that was requested to be removed in the initial 
diff, but rather "#include ", which I did remove.

I'm not sure why the "This looks wrong" gets attached to the "using" statement 
in later diffs.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:77
+
+  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return

tmroeder wrote:
> Eugene.Zelenko wrote:
> > Release Notes should include short description. One sentence is enough, but 
> > it'll good idea to keep it same as first statement of documentation.
> Would you like me to submit a patch that removes the second paragraph of this 
> description, then?
Yes. Documentation is created for details. But you still need to make its first 
sentence same as in Release Notes. See other checks and their Release Notes as 
example (in previous release branches).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder marked an inline comment as done.
tmroeder added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:77
+
+  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return

Eugene.Zelenko wrote:
> tmroeder wrote:
> > Eugene.Zelenko wrote:
> > > Release Notes should include short description. One sentence is enough, 
> > > but it'll good idea to keep it same as first statement of documentation.
> > Would you like me to submit a patch that removes the second paragraph of 
> > this description, then?
> Yes. Documentation is created for details. But you still need to make its 
> first sentence same as in Release Notes. See other checks and their Release 
> Notes as example (in previous release branches).
I'm sorry, I don't understand. What's the referent of "it" in "you still need 
to make its first sentence same as in Release Notes"? What first sentence needs 
to match the first sentence of the Release Notes?

If it's the header file documentation (MustCheckErrs.h), then as far as I can 
tell, the first paragraph of Release Notes is identical to the first paragraph 
of the .h file documentation, other than the double backquotes, which I didn't 
think belonged in a .h file comment.

What am I missing?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 211958.
Nathan-Huckleberry added a comment.

- Make filepath optionally user specified


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454

Files:
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.CallAndMessage.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DivideZero.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DynamicTypePropagation.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NonNullParamChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NullDereference.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.StackAddressEscape.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.UndefinedBinaryOperatorResult.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.VLASize.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.ArraySubscript.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Assign.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Branch.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.CapturedBlockVariable.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.UndefReturn.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.InnerPointer.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.Move.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDelete.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDeleteLeaks.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-deadcode.DeadStores.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullPassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableDereferenced.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullablePassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.UninitializedObject.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.VirtualCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.mpi.MPI-Checker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.OSObjectCStyleCast.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.GCDAntipattern.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.Padding.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.portability.UnixAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.API.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.MIG.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.NumberObjectConversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.OSObjectRetainCount.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.ObjCProperty.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.SecKeychainAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AtSync.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AutoreleaseWrite.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ClassRelease.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Dealloc.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.IncompatibleMethodTypes.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Loops.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.MissingSuperCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSAutoreleasePool.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSError.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NilArg.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NonNilReturnValue.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ObjCGenerics.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RetainCount.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.SelfInit.r

[PATCH] D65337: [clangd] Disallow extraction of expression-statements.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: SureYeaah.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

I split out the "extract parent instead of this" logic from the "this isn't
worth extracting" logic (now in eligibleForExtraction()), because I found it
hard to reason about.

While here, handle overloaded as well as builtin assignment operators.

Also this uncovered a bug in getCallExpr() which I fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65337

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -326,7 +326,7 @@
 a = [[2]];
   // while
   while(a < [[1]])
-[[a++]];
+a++;
   // do while
   do
 a = [[1]];
@@ -370,11 +370,14 @@
   // lambda
   auto lamb = [&[[a]], &[[b]]](int r = [[1]]) {return 1;}
   // assigment
-  [[a = 5]];
-  [[a >>= 5]];
-  [[a *= 5]];
+  xyz([[a = 5]]);
+  xyz([[a *= 5]]);
   // Variable DeclRefExpr
   a = [[b]];
+  // statement expression
+  [[xyz()]];
+  while (a)
+[[++a]];
   // label statement
   goto label;
   label:
@@ -417,11 +420,11 @@
   // Macros
   {R"cpp(#define PLUS(x) x++
  void f(int a) {
-   PLUS([[1+a]]);
+   int y = PLUS([[1+a]]);
  })cpp",
R"cpp(#define PLUS(x) x++
  void f(int a) {
-   auto dummy = PLUS(1+a); dummy;
+   auto dummy = PLUS(1+a); int y = dummy;
  })cpp"},
   // ensure InsertionPoint isn't inside a macro
   {R"cpp(#define LOOP(x) while (1) {a = x;}
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -368,32 +368,81 @@
   return Effect::applyEdit(Result);
 }
 
-// Find the CallExpr whose callee is an ancestor of the DeclRef
+// Find the CallExpr whose callee is the (possibly wrapped) DeclRef
 const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
-  // we maintain a stack of all exprs encountered while traversing the
-  // selectiontree because the callee of the callexpr can be an ancestor of the
-  // DeclRef. e.g. Callee can be an ImplicitCastExpr.
-  std::vector ExprStack;
-  for (auto *CurNode = DeclRef; CurNode; CurNode = CurNode->Parent) {
-const Expr *CurExpr = CurNode->ASTNode.get();
-if (const CallExpr *CallPar = CurNode->ASTNode.get()) {
-  // check whether the callee of the callexpr is present in Expr stack.
-  if (std::find(ExprStack.begin(), ExprStack.end(), CallPar->getCallee()) !=
-  ExprStack.end())
-return CurNode;
-  return nullptr;
-}
-ExprStack.push_back(CurExpr);
-  }
-  return nullptr;
+  const SelectionTree::Node &MaybeCallee = DeclRef->outerImplicit();
+  const SelectionTree::Node *MaybeCall = MaybeCallee.Parent;
+  if (!MaybeCall)
+return nullptr;
+  const CallExpr *CE =
+  llvm::dyn_cast_or_null(MaybeCall->ASTNode.get());
+  if (!CE)
+return nullptr;
+  if (CE->getCallee() != MaybeCallee.ASTNode.get())
+return nullptr;
+  return MaybeCall;
 }
 
-// check if Expr can be assigned to a variable i.e. is non-void type
-bool canBeAssigned(const SelectionTree::Node *ExprNode) {
-  const clang::Expr *Expr = ExprNode->ASTNode.get();
-  if (const Type *ExprType = Expr->getType().getTypePtrOrNull())
-// FIXME: check if we need to cover any other types
-return !ExprType->isVoidType();
+// Returns true if Inner (which is a direct child of Outer) is appearing as
+// a statement rather than an expression whose value can be used.
+bool childExprIsStmt(const Stmt *Outer, const Expr *Inner) {
+  if (!Outer || !Inner)
+return false;
+  Outer->dumpColor();
+  // Blacklist the most common places where an expr can appear but be unused.
+  if (llvm::isa(Outer))
+return true;
+  if (llvm::isa(Outer))
+return true;
+  // Control flow statements use condition etc, but not the body.
+  if (const auto* WS = llvm::dyn_cast(Outer))
+return Inner == WS->getBody();
+  if (const auto* DS = llvm::dyn_cast(Outer))
+return Inner == DS->getBody();
+  if (const auto* FS = llvm::dyn_cast(Outer))
+return Inner == FS->getBody();
+  if (const auto* FS = llvm::dyn_cast(Outer))
+return Inner == FS->getBody();
+  if (const auto* IS = llvm::dyn_cast(Oute

[PATCH] D64932: [Parser] Emit descriptive diagnostic for misplaced pragma

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64932



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

[PATCH] D65302: [clang][docs][release notes] mention asm goto support

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks!

Do you want to write something more general for the release notes about the 
status of Clang and the Linux kernel?




Comment at: clang/docs/ReleaseNotes.rst:116
+  control flow from inline assembly. The main consumer of this construct is the
+  Linux kernel and glib. There a few long tail bugs in Clang's integrated
+  assembler and IfConverter still being fixed related to the use of

nickdesaulniers wrote:
> I wasn't sure the best way to state something along the lines of "support is 
> in beta shape, plz report bugz?"
How about starting with "Initial support has been added for ...", and ending 
with "There are still unsupported corner cases in Clang's integrated assembler 
and in IfConverter. Please file bugs for any issues you run into." ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65302



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

[PATCH] D36767: Implement the standard for timeouts for std::condition_variable

[Tom via Phabricator via cfe-commits]

tomcherry abandoned this revision.
tomcherry added a comment.
Herald added a subscriber: jfb.

See https://reviews.llvm.org/D65339


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

https://reviews.llvm.org/D36767



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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py:113
+  file_path_help = ("Path to Checkers.td, defaults to 
../../../../clang/include/clang/StaticAnalyzer/Checkers/")
+  parse.add_argument("file", type=str, help=file_path_help, nargs='?', 
default='../../../../clang/include/clang/StaticAnalyzer/Checkers/')
+  args = parse.parse_args()

I'm not super enthusiastic about this approach because the default is known to 
be wrong for supported ways of building Clang. It does give us some way 
forward, but getting the default wrong for some number of our developers is 
unfortunate.

Why not test the path you list there (which should be good for monorepo) and if 
that isn't found, test `../../../../../include/clang/StaticAnalyzer/Checkers/` 
(which should be good for in-tree builds)? This way the arguments only need to 
be supplied if you have a truly odd directory layout that we can't otherwise 
guess at.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry added a comment.

I agree that parsing according to attribute name/type is not a good solution.

It sounds like we have narrowed it down to two choices:
Do we want to follow the gcc method of parsing once and falling back if parsing 
fails?
Do we want to parse attributes first and then wait until we see a 
decl-specifier (breaking the implicit int case)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

r367134 - [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

[Nathan Huckleberry via cfe-commits]

Author: nathan-huckleberry
Date: Fri Jul 26 10:29:35 2019
New Revision: 367134

URL: http://llvm.org/viewvc/llvm-project?rev=367134&view=rev
Log:
[Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

Summary:
Do not automatically report self references of structs in statement expression
as warnings. Instead wait for uninitialized cfg analysis.
https://bugs.llvm.org/show_bug.cgi?id=42604

Reviewers: aaron.ballman, rsmith, nickdesaulniers

Reviewed By: aaron.ballman, nickdesaulniers

Subscribers: nathanchance, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=367134&r1=367133&r2=367134&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jul 26 10:29:35 2019
@@ -11257,9 +11257,12 @@ void Sema::AddInitializerToDecl(Decl *Re
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
-  if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
-  VDecl->getType()->isReferenceType()) {
-CheckSelfReference(*this, RealDecl, Init, DirectInit);
+  // This is undefined behavior in C++, but valid in C.
+  if (getLangOpts().CPlusPlus) {
+if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
+VDecl->getType()->isReferenceType()) {
+  CheckSelfReference(*this, RealDecl, Init, DirectInit);
+}
   }
 
   // If the type changed, it means we had an incomplete type that was

Added: cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c?rev=367134&view=auto
==
--- cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c (added)
+++ cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c Fri Jul 26 
10:29:35 2019
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void init(int *);
+
+void foo(void) {
+  int i = ({
+init(&i);
+i;
+  });
+}
+
+void foo_bad(void) {
+  int i = ({
+int z = i; // expected-warning{{variable 'i' is uninitialized when used 
within its own initialization}}
+init(&i);
+i;
+  });
+}
+
+struct widget {
+  int x, y;
+};
+void init2(struct widget *);
+
+void bar(void) {
+  struct widget my_widget = ({
+init2(&my_widget);
+my_widget;
+  });
+  struct widget a = (init2(&a), a);
+}
+
+void bar_bad(void) {
+  struct widget my_widget = ({
+struct widget z = my_widget; // expected-warning{{variable 'my_widget' is 
uninitialized when used within its own initialization}}
+int x = my_widget.x; //FIXME: There should be an uninitialized 
warning here
+init2(&my_widget);
+my_widget;
+  });
+}
+
+void baz(void) {
+  struct widget a = ({
+struct widget b = ({
+  b = a; // expected-warning{{variable 'a' is uninitialized when used 
within its own initialization}}
+});
+a;
+  });
+}
+
+void f(void) {
+  struct widget *a = ({
+init2(a); // expected-warning{{variable 'a' is uninitialized when used 
within its own initialization}}
+a;
+  });
+}


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

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

[Nathan Huckleberry via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367134: [Sema] Fix -Wuninitialized for struct assignment 
from GNU C statement expression (authored by Nathan-Huckleberry, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64678?vs=211545&id=211966#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64678

Files:
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c


Index: cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
===
--- cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
+++ cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void init(int *);
+
+void foo(void) {
+  int i = ({
+init(&i);
+i;
+  });
+}
+
+void foo_bad(void) {
+  int i = ({
+int z = i; // expected-warning{{variable 'i' is uninitialized when used 
within its own initialization}}
+init(&i);
+i;
+  });
+}
+
+struct widget {
+  int x, y;
+};
+void init2(struct widget *);
+
+void bar(void) {
+  struct widget my_widget = ({
+init2(&my_widget);
+my_widget;
+  });
+  struct widget a = (init2(&a), a);
+}
+
+void bar_bad(void) {
+  struct widget my_widget = ({
+struct widget z = my_widget; // expected-warning{{variable 'my_widget' is 
uninitialized when used within its own initialization}}
+int x = my_widget.x; //FIXME: There should be an uninitialized 
warning here
+init2(&my_widget);
+my_widget;
+  });
+}
+
+void baz(void) {
+  struct widget a = ({
+struct widget b = ({
+  b = a; // expected-warning{{variable 'a' is uninitialized when used 
within its own initialization}}
+});
+a;
+  });
+}
+
+void f(void) {
+  struct widget *a = ({
+init2(a); // expected-warning{{variable 'a' is uninitialized when used 
within its own initialization}}
+a;
+  });
+}
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -11257,9 +11257,12 @@
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
-  if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
-  VDecl->getType()->isReferenceType()) {
-CheckSelfReference(*this, RealDecl, Init, DirectInit);
+  // This is undefined behavior in C++, but valid in C.
+  if (getLangOpts().CPlusPlus) {
+if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() ||
+VDecl->getType()->isReferenceType()) {
+  CheckSelfReference(*this, RealDecl, Init, DirectInit);
+}
   }
 
   // If the type changed, it means we had an incomplete type that was


Index: cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
===
--- cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
+++ cfe/trunk/test/Sema/warn-uninitialized-statement-expression.c
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
+void init(int *);
+
+void foo(void) {
+  int i = ({
+init(&i);
+i;
+  });
+}
+
+void foo_bad(void) {
+  int i = ({
+int z = i; // expected-warning{{variable 'i' is uninitialized when used within its own initialization}}
+init(&i);
+i;
+  });
+}
+
+struct widget {
+  int x, y;
+};
+void init2(struct widget *);
+
+void bar(void) {
+  struct widget my_widget = ({
+init2(&my_widget);
+my_widget;
+  });
+  struct widget a = (init2(&a), a);
+}
+
+void bar_bad(void) {
+  struct widget my_widget = ({
+struct widget z = my_widget; // expected-warning{{variable 'my_widget' is uninitialized when used within its own initialization}}
+int x = my_widget.x; //FIXME: There should be an uninitialized warning here
+init2(&my_widget);
+my_widget;
+  });
+}
+
+void baz(void) {
+  struct widget a = ({
+struct widget b = ({
+  b = a; // expected-warning{{variable 'a' is uninitialized when used within its own initialization}}
+});
+a;
+  });
+}
+
+void f(void) {
+  struct widget *a = ({
+init2(a); // expected-warning{{variable 'a' is uninitialized when used within its own initialization}}
+a;
+  });
+}
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -11257,9 +11257,12 @@
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
-  if (!VDecl->ha

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:815
+  /// code generation.
+  void emitUDMapperArrayInitOrDel(CodeGenFunction &MapperCGF,
+  llvm::Value *Handle, llvm::Value *BasePtr,

Seems to me, this function is used only in `emitUserDefinedMapper`. I think you 
can make it static local in the CGOpenMPRuntime.cpp and do not expose it in the 
interface.


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

https://reviews.llvm.org/D59474



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

Re: [clang-tools-extra] r367112 - [clangd] Fix background index not triggering on windows due to case mismatch.

[Hans Wennborg via cfe-commits]

Merged to release_90 in r367135.

On Fri, Jul 26, 2019 at 7:06 AM Sam McCall via cfe-commits
 wrote:
>
> Author: sammccall
> Date: Fri Jul 26 07:07:11 2019
> New Revision: 367112
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367112&view=rev
> Log:
> [clangd] Fix background index not triggering on windows due to case mismatch.
>
> Summary:
> This isn't a general fix to all paths where we assume case-sensitivity, it's
> a minimally-invasive fix targeting the llvm 9 branch.
>
> Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D65320
>
> Modified:
> clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
> clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
>
> Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=367112&r1=367111&r2=367112&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
> +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Fri Jul 26 
> 07:07:11 2019
> @@ -117,20 +117,41 @@ DirectoryBasedGlobalCompilationDatabase:
>return None;
>  }
>
> -std::pair
> +// For platforms where paths are case-insensitive (but case-preserving),
> +// we need to do case-insensitive comparisons and use lowercase keys.
> +// FIXME: Make Path a real class with desired semantics instead.
> +//This class is not the only place this problem exists.
> +// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
> +
> +static std::string maybeCaseFoldPath(PathRef Path) {
> +#if defined(_WIN32) || defined(__APPLE__)
> +  return Path.lower();
> +#else
> +  return Path;
> +#endif
> +}
> +
> +static bool pathEqual(PathRef A, PathRef B) {
> +#if defined(_WIN32) || defined(__APPLE__)
> +  return A.equals_lower(B);
> +#else
> +  return A == B;
> +#endif
> +}
> +
> +DirectoryBasedGlobalCompilationDatabase::CachedCDB &
>  DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) 
> const {
>// FIXME(ibiryukov): Invalidate cached compilation databases on changes
> -  auto CachedIt = CompilationDatabases.find(Dir);
> -  if (CachedIt != CompilationDatabases.end())
> -return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
> -  std::string Error = "";
> -
> -  CachedCDB Entry;
> -  Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
> -  auto Result = Entry.CDB.get();
> -  CompilationDatabases[Dir] = std::move(Entry);
> -
> -  return {Result, false};
> +  // FIXME(sammccall): this function hot, avoid copying key when hitting 
> cache.
> +  auto Key = maybeCaseFoldPath(Dir);
> +  auto R = CompilationDatabases.try_emplace(Key);
> +  if (R.second) { // Cache miss, try to load CDB.
> +CachedCDB &Entry = R.first->second;
> +std::string Error = "";
> +Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
> +Entry.Path = Dir;
> +  }
> +  return R.first->second;
>  }
>
>  llvm::Optional
> @@ -139,38 +160,41 @@ DirectoryBasedGlobalCompilationDatabase:
>assert(llvm::sys::path::is_absolute(Request.FileName) &&
>   "path must be absolute");
>
> +  bool ShouldBroadcast = false;
>CDBLookupResult Result;
> -  bool SentBroadcast = false;
>
>{
>  std::lock_guard Lock(Mutex);
> +CachedCDB *Entry = nullptr;
>  if (CompileCommandsDir) {
> -  std::tie(Result.CDB, SentBroadcast) =
> -  getCDBInDirLocked(*CompileCommandsDir);
> -  Result.PI.SourceRoot = *CompileCommandsDir;
> +  Entry = &getCDBInDirLocked(*CompileCommandsDir);
>  } else {
>// Traverse the canonical version to prevent false positives. i.e.:
>// src/build/../a.cc can detect a CDB in /src/build if not 
> canonicalized.
> +  // FIXME(sammccall): this loop is hot, use a union-find-like structure.
>actOnAllParentDirectories(removeDots(Request.FileName),
> -[this, &SentBroadcast, &Result](PathRef 
> Path) {
> -  std::tie(Result.CDB, SentBroadcast) =
> -  getCDBInDirLocked(Path);
> -  Result.PI.SourceRoot = Path;
> -  return Result.CDB != nullptr;
> +[&](PathRef Path) {
> +  Entry = &getCDBInDirLocked(Path);
> +  return Entry->CDB != nullptr;
>  });
>  }
>
> -if (!Result.CDB)
> +if (!Entry || !Entry->CDB)
>return llvm::None;
>
>  // Mark CDB as broadcasted to make sure discovery is performed once.
> -if (Request.ShouldBroadcast && !SentBroadcast)
> -  CompilationDatabases[Result.PI.SourceRoot].SentBroadcast 

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 211967.
Nathan-Huckleberry added a comment.

- Add in-tree as possible default location


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454

Files:
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.CallAndMessage.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DivideZero.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DynamicTypePropagation.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NonNullParamChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NullDereference.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.StackAddressEscape.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.UndefinedBinaryOperatorResult.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.VLASize.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.ArraySubscript.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Assign.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Branch.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.CapturedBlockVariable.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.UndefReturn.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.InnerPointer.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.Move.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDelete.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDeleteLeaks.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-deadcode.DeadStores.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullPassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableDereferenced.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullablePassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.UninitializedObject.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.VirtualCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.mpi.MPI-Checker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.OSObjectCStyleCast.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.GCDAntipattern.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.Padding.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.portability.UnixAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.API.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.MIG.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.NumberObjectConversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.OSObjectRetainCount.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.ObjCProperty.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.SecKeychainAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AtSync.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AutoreleaseWrite.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ClassRelease.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Dealloc.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.IncompatibleMethodTypes.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Loops.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.MissingSuperCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSAutoreleasePool.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSError.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NilArg.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NonNilReturnValue.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ObjCGenerics.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RetainCount.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.SelfInit.

[PATCH] D65110: [NewPM] Run avx*-builtins.c tests under the new pass manager only

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65110



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

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp:47-52
+
+  Namespaces
+  Records
+  Functions
+  OneFunction
+

juliehockett wrote:
> Formatting is a bit weird for sublists
Fixed by D65005. It will be properly formatted after rebasing.


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

https://reviews.llvm.org/D65030



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, caomhin.
Herald added subscribers: cfe-commits, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

This patch adds support for the close map modifier in Clang.

This ensures that the new map type is marked and passed to the OpenMP runtime 
appropriately.


Repository:
  rC Clang

https://reviews.llvm.org/D65341

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


Index: test/OpenMP/target_data_codegen.cpp
===
--- test/OpenMP/target_data_codegen.cpp
+++ test/OpenMP/target_data_codegen.cpp
@@ -283,4 +283,13 @@
   {++arg;}
 }
 #endif
+///==///
+ RUN: %clang_cc1 -DCK4 -verify -fopenmp -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-o - | FileCheck %s --check-prefix CK4
+#ifdef CK4
+void test_close_modifier(int arg) {
+  // CK4: private unnamed_addr constant [1 x i64] [i64 1059]
+  #pragma omp target data map(close,tofrom: arg)
+  {++arg;}
+}
+#endif
 #endif
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7087,6 +7087,9 @@
 OMP_MAP_LITERAL = 0x100,
 /// Implicit map
 OMP_MAP_IMPLICIT = 0x200,
+/// Close is a hint to the runtime to allocate memory close to
+/// the target device.
+OMP_MAP_CLOSE = 0x400,
 /// The 16 MSBs of the flags indicate whether the entry is member of some
 /// struct/class.
 OMP_MAP_MEMBER_OF = 0x,
@@ -7255,6 +7258,9 @@
 if (llvm::find(MapModifiers, OMPC_MAP_MODIFIER_always)
 != MapModifiers.end())
   Bits |= OMP_MAP_ALWAYS;
+if (llvm::find(MapModifiers, OMPC_MAP_MODIFIER_close)
+!= MapModifiers.end())
+  Bits |= OMP_MAP_CLOSE;
 return Bits;
   }
 
@@ -7686,7 +7692,7 @@
 // then we reset the TO/FROM/ALWAYS/DELETE flags.
 if (IsPointer)
   Flags &= ~(OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_ALWAYS |
- OMP_MAP_DELETE);
+ OMP_MAP_DELETE | OMP_MAP_CLOSE);
 
 if (ShouldBeMemberOf) {
   // Set placeholder value MEMBER_OF= to indicate that the flag


Index: test/OpenMP/target_data_codegen.cpp
===
--- test/OpenMP/target_data_codegen.cpp
+++ test/OpenMP/target_data_codegen.cpp
@@ -283,4 +283,13 @@
   {++arg;}
 }
 #endif
+///==///
+ RUN: %clang_cc1 -DCK4 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -o - | FileCheck %s --check-prefix CK4
+#ifdef CK4
+void test_close_modifier(int arg) {
+  // CK4: private unnamed_addr constant [1 x i64] [i64 1059]
+  #pragma omp target data map(close,tofrom: arg)
+  {++arg;}
+}
+#endif
 #endif
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7087,6 +7087,9 @@
 OMP_MAP_LITERAL = 0x100,
 /// Implicit map
 OMP_MAP_IMPLICIT = 0x200,
+/// Close is a hint to the runtime to allocate memory close to
+/// the target device.
+OMP_MAP_CLOSE = 0x400,
 /// The 16 MSBs of the flags indicate whether the entry is member of some
 /// struct/class.
 OMP_MAP_MEMBER_OF = 0x,
@@ -7255,6 +7258,9 @@
 if (llvm::find(MapModifiers, OMPC_MAP_MODIFIER_always)
 != MapModifiers.end())
   Bits |= OMP_MAP_ALWAYS;
+if (llvm::find(MapModifiers, OMPC_MAP_MODIFIER_close)
+!= MapModifiers.end())
+  Bits |= OMP_MAP_CLOSE;
 return Bits;
   }
 
@@ -7686,7 +7692,7 @@
 // then we reset the TO/FROM/ALWAYS/DELETE flags.
 if (IsPointer)
   Flags &= ~(OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_ALWAYS |
- OMP_MAP_DELETE);
+ OMP_MAP_DELETE | OMP_MAP_CLOSE);
 
 if (ShouldBeMemberOf) {
   // Set placeholder value MEMBER_OF= to indicate that the flag
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

[Ziang Wan via Phabricator via cfe-commits]

ziangwan marked 4 inline comments as done.
ziangwan added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:2188-2190
+/// shared + exclusive = exclusive
+/// generic + exclusive = exclusive
+/// generic + shared = shared

aaronpuchert wrote:
> What do these lines mean? That we accept if a lock is shared in one branch 
> and exclusive in the other, and that we make it exclusive after the merge 
> point?
Yes. If at CFG merge point, one path holds type1 lock and the other path holds 
type2 lock.

We do a type1 + type2 = merged_type and issue warnings if we are doing shared + 
exclusive merge.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:2219-2221
+if (LDat1->kind() == LK_Generic || LDat2->kind() == LK_Generic) {
+  // No warning is issued in this case.
+  if (Modify && LDat1->kind() == LK_Generic) {

nickdesaulniers wrote:
> The double check of `LDat1->kind() == LK_Generic` is fishy to me.  
> Particularly the case where `LDat1->kind() == LK_Generic` is false but 
> `LDat2->kind() == LK_Generic` is true.
> 
> This might be clearer as:
> ```
> if (LDat2->kind() == LK_Generic)
>   continue;
> else if (LDat1->kind() == LK_Generic && Modify)
>   *Iter1 = Fact;
> else {
>   ...
> ```
> Or is there something else to this logic I'm missing?
I think your suggestion is to refactor the if statements. What I am thinking is 
that there are two cases.
1. One of the two locks is generic
  * If so, then take the type of the other non-generic lock (shared or 
exclusive).
2. Neither of the two locks is generic.

My if statement is trying express that. I am afraid the refactoring is going to 
lose the logic (as stated in my comment above).



Comment at: clang/test/SemaCXX/thread-safety-annotations.h:47
+// Enable thread safety attributes only with clang.
+// The attributes can be safely erased when compiling with other compilers.
+#if defined(__clang__) && (!defined(SWIG))

nickdesaulniers wrote:
> Is this test suite run with other compilers? If not, I think we can remove 
> the case?
Yeah, you are right. I just copied the header definitions from clang thread 
safety analysis doc.



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:135-140
+  if (condition) {
+assertNotHeld(); // expected-warning {{mutex '!mu' is acquired exclusively 
and shared in the same scope}}
+  } else {
+mu.Lock();
+mu.Unlock(); // expected-warning {{the other acquisition of mutex '!mu' is 
here}}
+  }

aaronpuchert wrote:
> Why would I want these warnings here? This code seems fine to me.
> 
> However, I don't see why we don't get `acquiring mutex 'mu' requires negative 
> capability '!mu'` at line 138, or does that disappear because of the 
> assertion?
Showing `acquiring mutex 'mu' requires negative capability '!mu'` is not in the 
scope of this patch. Please notice thread safety analysis is still under 
development.

The reason is that, in one path we have `ASSERT_SHARED_CAPABILITY(!mu)`, and in 
the other path we have `RELEASE(mu)`. The assertion leads to negative shared 
capability but the release leads to negative exclusive capability. A merge of 
the two capabilities (merging "I am not trying to read" versus "I am not trying 
to write") leads to a warning.

Without my patch, clang will issue a warning for the merge point in test1() but 
not the merge point in test2().


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7695
   Flags &= ~(OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_ALWAYS |
- OMP_MAP_DELETE);
+ OMP_MAP_DELETE | OMP_MAP_CLOSE);
 

Why?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D65309: [clang-format] Fix style of css file paths

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 211971.
DiegoAstiazaran added a comment.

Add comment.


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

https://reviews.llvm.org/D65309

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp


Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -110,34 +110,23 @@
   ClangDocContext CDCtx = getClangDocContext();
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
-  SmallString<16> PathToF;
-  llvm::sys::path::native("../../../path/to/F.html", PathToF);
-  SmallString<16> PathToInt;
-  llvm::sys::path::native("../int.html", PathToInt);
-  SmallString<16> PathToSylesheet;
-  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
-  PathToSylesheet);
   std::string Expected = R"raw(
 
 class r
-
+
 
   class r
   Defined at line 10 of test.cpp
   
 Inherits from 
-F
+F
 , G
   
   Members
   
 
   private 
-  int
+  int
X
 
   
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -231,6 +231,8 @@
 SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(StylesheetPath,
 llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);
 Out.emplace_back(std::move(LinkNode));
   }


Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -110,34 +110,23 @@
   ClangDocContext CDCtx = getClangDocContext();
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
-  SmallString<16> PathToF;
-  llvm::sys::path::native("../../../path/to/F.html", PathToF);
-  SmallString<16> PathToInt;
-  llvm::sys::path::native("../int.html", PathToInt);
-  SmallString<16> PathToSylesheet;
-  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
-  PathToSylesheet);
   std::string Expected = R"raw(
 
 class r
-
+
 
   class r
   Defined at line 10 of test.cpp
   
 Inherits from 
-F
+F
 , G
   
   Members
   
 
   private 
-  int
+  int
X
 
   
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -231,6 +231,8 @@
 SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(StylesheetPath,
 llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);
 Out.emplace_back(std::move(LinkNode));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r367137 - [clang-format] Fix style of css file paths

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Fri Jul 26 11:02:42 2019
New Revision: 367137

URL: http://llvm.org/viewvc/llvm-project?rev=367137&view=rev
Log:
[clang-format] Fix style of css file paths

CSS files included in HTML should have a path in posix style, it should
not be different for Windows.

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

Modified:
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=367137&r1=367136&r2=367137&view=diff
==
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Fri Jul 26 11:02:42 2019
@@ -231,6 +231,8 @@ genStylesheetsHTML(StringRef InfoPath, c
 SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(StylesheetPath,
 llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);
 Out.emplace_back(std::move(LinkNode));
   }

Modified: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp?rev=367137&r1=367136&r2=367137&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp Fri Jul 
26 11:02:42 2019
@@ -110,34 +110,23 @@ TEST(HTMLGeneratorTest, emitRecordHTML)
   ClangDocContext CDCtx = getClangDocContext();
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
-  SmallString<16> PathToF;
-  llvm::sys::path::native("../../../path/to/F.html", PathToF);
-  SmallString<16> PathToInt;
-  llvm::sys::path::native("../int.html", PathToInt);
-  SmallString<16> PathToSylesheet;
-  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
-  PathToSylesheet);
   std::string Expected = R"raw(
 
 class r
-
+
 
   class r
   Defined at line 10 of test.cpp
   
 Inherits from 
-F
+F
 , G
   
   Members
   
 
   private 
-  int
+  int
X
 
   


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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh marked an inline comment as done.
lildmh added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:815
+  /// code generation.
+  void emitUDMapperArrayInitOrDel(CodeGenFunction &MapperCGF,
+  llvm::Value *Handle, llvm::Value *BasePtr,

ABataev wrote:
> Seems to me, this function is used only in `emitUserDefinedMapper`. I think 
> you can make it static local in the CGOpenMPRuntime.cpp and do not expose it 
> in the interface.
`emitUserDefinedMapper` needs to call `createRuntimeFunction` of 
`CGOpenMPRuntime`, which is private.

Which one do you think is better, make `createRuntimeFunction` public, or have 
`emitUserDefinedMapper` not defined in `CGOpenMPRuntime`? It seems to me that 
they are similar


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

https://reviews.llvm.org/D59474



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

[PATCH] D65309: [clang-format] Fix style of css file paths

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367137: [clang-format] Fix style of css file paths (authored 
by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65309?vs=211971&id=211972#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65309

Files:
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp


Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -110,34 +110,23 @@
   ClangDocContext CDCtx = getClangDocContext();
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
-  SmallString<16> PathToF;
-  llvm::sys::path::native("../../../path/to/F.html", PathToF);
-  SmallString<16> PathToInt;
-  llvm::sys::path::native("../int.html", PathToInt);
-  SmallString<16> PathToSylesheet;
-  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
-  PathToSylesheet);
   std::string Expected = R"raw(
 
 class r
-
+
 
   class r
   Defined at line 10 of test.cpp
   
 Inherits from 
-F
+F
 , G
   
   Members
   
 
   private 
-  int
+  int
X
 
   
Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -231,6 +231,8 @@
 SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(StylesheetPath,
 llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);
 Out.emplace_back(std::move(LinkNode));
   }


Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -110,34 +110,23 @@
   ClangDocContext CDCtx = getClangDocContext();
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
-  SmallString<16> PathToF;
-  llvm::sys::path::native("../../../path/to/F.html", PathToF);
-  SmallString<16> PathToInt;
-  llvm::sys::path::native("../int.html", PathToInt);
-  SmallString<16> PathToSylesheet;
-  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
-  PathToSylesheet);
   std::string Expected = R"raw(
 
 class r
-
+
 
   class r
   Defined at line 10 of test.cpp
   
 Inherits from 
-F
+F
 , G
   
   Members
   
 
   private 
-  int
+  int
X
 
   
Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -231,6 +231,8 @@
 SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(StylesheetPath,
 llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(StylesheetPath, llvm::sys::path::Style::posix);
 LinkNode->Attributes.try_emplace("href", StylesheetPath);
 Out.emplace_back(std::move(LinkNode));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

[Jan Korous via Phabricator via cfe-commits]

jkorous abandoned this revision.
jkorous added a comment.

Abandoned in favor of
https://reviews.llvm.org/D65301


Repository:
  rC Clang

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

https://reviews.llvm.org/D61104



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

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

[Jan Korous via Phabricator via cfe-commits]

jkorous abandoned this revision.
jkorous added a comment.

Abandoned in favor of https://reviews.llvm.org/D65301


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

https://reviews.llvm.org/D61103



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

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi @jdenny, thanks for the warning! I think having the test disabled is fine - 
the main upside I see is that we won't check it fails over and over on our CI 
systems.
Could you please mention the test/reproducer in those FIXMEs?
Otherwise LGTM.




Comment at: clang/unittests/Rewrite/RewriteBufferTest.cpp:17
 
+#define EXPECT_OUTPUT(Buf, Output) EXPECT_EQ(Output, writeOutput(Buf))
+

I think in case you really wanted to have an "XFAIL", you could have just used 
`EXPECT_NE` here.


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

https://reviews.llvm.org/D61466



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

[PATCH] D65000: [ARM] Set default alignment to 64bits

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp:9
+// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16
+#include 
+

This fails on some bots:

http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/26891/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aexception-alignment.cpp

```
In file included from 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/test/CodeGen/ARM/exception-alignment.cpp:9:
In file included from 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/lib/clang/10.0.0/include/arm_neon.h:31:
In file included from 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/lib/clang/10.0.0/include/stdint.h:52:
In file included from /usr/include/stdint.h:26:
In file included from /usr/include/bits/libc-header-start.h:33:
In file included from /usr/include/features.h:452:
/usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found
# include 
  ^~~~
1 error generated.
FileCheck error: '-' is empty.
FileCheck command line:  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/bin/FileCheck 
--check-prefixes=CHECK,A16 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/test/CodeGen/ARM/exception-alignment.cpp

--
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65000



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

[PATCH] D64932: [Parser] Emit descriptive diagnostic for misplaced pragma

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Basic/TokenKinds.cpp:55
+return std::strncmp(#X, "pragma_", sizeof("pragma_") - 1) == 0;
+#include "clang/Basic/TokenKinds.def"
+  default:

The right way to do this is to make a `PRAGMA_ANNOTATION` macro in 
`TokenKinds.def` that defaults to `ANNOTATION`, like that file already does 
with e.g. `CXX11_KEYWORD`.  But for future reference, there's also a 
`StringRef::startsWith`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64932



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

[PATCH] D64811: Warn when NumParams overflows

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/AST/Type.h:1523
 
+  enum { NumParamsBits = 16 };
+

This should probably go in `FunctionTypeBitfields`.



Comment at: clang/lib/Parse/ParseDecl.cpp:6673
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {

We don't usually cite bugs like this in the main source code except as "there's 
a bug with this, here's where we tracking fixing it".  The comment explains the 
purpose of the diagnostic well enough (but please include a period).

Can you just drop the excess arguments here instead of cutting off parsing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64811



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

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D61466#1602917 , @jkorous wrote:

> Hi @jdenny, thanks for the warning!


Hi.  Thanks for the review.

> I think having the test disabled is fine - the main upside I see is that we 
> won't check it fails over and over on our CI systems.

In an inline comment, you also mentioned the alternative of replacing 
`EXPECT_EQ` with `EXPECT_NE`.  Neither solution is the XFAIL I'm used to (from 
lit for example).

I chose disabling purely because I saw some other unit tests do this.  What do 
people normally recommend for llvm unit tests?

> Could you please mention the test/reproducer in those FIXMEs?

Will do.


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

https://reviews.llvm.org/D61466



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:815
+  /// code generation.
+  void emitUDMapperArrayInitOrDel(CodeGenFunction &MapperCGF,
+  llvm::Value *Handle, llvm::Value *BasePtr,

lildmh wrote:
> ABataev wrote:
> > Seems to me, this function is used only in `emitUserDefinedMapper`. I think 
> > you can make it static local in the CGOpenMPRuntime.cpp and do not expose 
> > it in the interface.
> `emitUserDefinedMapper` needs to call `createRuntimeFunction` of 
> `CGOpenMPRuntime`, which is private.
> 
> Which one do you think is better, make `createRuntimeFunction` public, or 
> have `emitUserDefinedMapper` not defined in `CGOpenMPRuntime`? It seems to me 
> that they are similar
Then make `emitUDMapperArrayInitOrDel` private instead.


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

https://reviews.llvm.org/D59474



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/test/CodeGen/PR42672.c:50
+  struct {
+long long int v1, v2, v3, v4;
+  } str;

glider wrote:
> The acceptable size actually depends on the target platform. Not sure we can 
> test for all of them, and we'll probably need to restrict this test to e.g. 
> x86
The interesting case for a 32-byte struct would actually be something like 
`"=x"(str)`... which currently passes the clang frontend, since 32 is a legal 
size for that constraint (although it eventually fails in the backend).



Comment at: clang/test/CodeGen/PR42672.c:3
+// RUN: %clang_cc1 -USTRUCT -emit-llvm %s -o - | FileCheck %s 
--check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE -emit-llvm %s 
-o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t

Might as well just make all these tests use an x86 triple; I don't really want 
to speculate how an unknown target will react to any specific size.



Comment at: clang/test/CodeGen/PR42672.c:40
+unsigned short first;
+unsigned char second;
+  } str;

This isn't a three-byte struct; it's a four-byte struct where one of the bytes 
is only used for padding. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

When I run the script locally, I still get issues with `subprocess.run()`.

  c:\llvm\tools\clang\tools\extra\docs\clang-tidy\checks>python 
gen-static-analyzer-docs.py
  Traceback (most recent call last):
File "gen-static-analyzer-docs.py", line 148, in 
  main()
File "gen-static-analyzer-docs.py", line 137, in main
  checkers = get_checkers(file_path)
File "gen-static-analyzer-docs.py", line 22, in get_checkers
  result = subprocess.run(["llvm-tblgen", "--dump-json", "-I",
  AttributeError: 'module' object has no attribute 'run'




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py:120-123
+  if(os.path.exists(default_path_in_tree)):
+default_path = default_path_in_tree
+  if(os.path.exists(default_path_monorepo)):
+default_path = default_path_monorepo

Swap the order of these and use an `elif` so we don't set the path and then 
overwrite it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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

[PATCH] D64811: Warn when NumParams overflows

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:6673
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {

rjmccall wrote:
> We don't usually cite bugs like this in the main source code except as 
> "there's a bug with this, here's where we tracking fixing it".  The comment 
> explains the purpose of the diagnostic well enough (but please include a 
> period).
> 
> Can you just drop the excess arguments here instead of cutting off parsing?
Or actually — let's move this out of the parser entirely, because (unlike 
prototype scope depth) we actually need to diagnose it in Sema as well because 
of variadic templates.  `Sema::BuildFunctionType` seems like the right place.

Also, can you `static_assert` that function declarations support at least as 
many parameter declarations as we allow in types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64811



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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added a comment.

Thanks for your review >>! In D62731#1601408 
, @kpn wrote:

> In D62731#1601310 , @mibintc wrote:
> 
>> I think it would be convenient to have an "unset" setting for the different 
>> constrained modes, otherwise you need a boolean that says "no value was 
>> provided for this option".  But i'm a frontend person so I may need to have 
>> my attitude adjusted.
> 
> 
> What "different constrained modes"? The IRBuilder is either in constrained 
> mode or it isn't. In constrained mode the exception behavior and rounding 
> mode both have defaults, and those defaults can be changed individually 
> without affecting the other setting. The current defaults can also be 
> retrieved if you need something for a function call where you don't want to 
> change it but need an argument anyway. When do you need this "no value 
> provided" setting?

I'm going to rewrite this

> Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional 
> appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know 
> who has opinions on how command line options should be handled.

I'd like to fix it more before I add more reviewers

> Do we want the Unix driver to be compatible with gcc? Maybe, maybe not. 
> Opinions, anyone?

Oh, I think you mean something more like the gnuish -fno-except or maybe 
-fp-model=no-except? instead of -fp-model=except- ok we can get that sorted.

> The documentation request from lebedev.ri isn't in this ticket yet.
> 
> Also, for future historical research purposes I'd cut and paste the relevant 
> portions of outside web pages (like intel.com's) and post them somewhere 
> llvm-ish where they are findable. This ticket, for example, is a good place. 
> Web sites gets reorganized or vanish in full or in part. It's helpful to have 
> some insulation from that over time. I've had to fix compiler bugs that 
> actually were 25 years old before. Yes, 25 years old. Being able to do the 
> research is very helpful.

That's a good idea thanks

> Oh, it may be useful to know that constrained floating point and the 
> FastMathFlags are not mutually exclusive. I don't know if that matters here 
> or not, but you did mention FastMathFlags.

Yes i'm not sure how the fast math command line optoins should interact with 
the fp-model options, i'll have to dig into that.




Comment at: llvm/lib/IR/FPState.cpp:1
+#include "llvm/IR/FPState.h"
+#include "llvm/IR/IRBuilder.h"

arsenm wrote:
> Missing license header
Thanks @arsenm in the end i believe i won't be adding this file



Comment at: llvm/lib/IR/FPState.cpp:73
+  if (IsConstrainedExcept && !IsConstrainedRounding) {
+// If the rounding mode isn't set explicitly above, then use ebToNearest
+// as the value when the constrained intrinsic is created

arsenm wrote:
> eb?
I mean, if the user requested constrained exception via the option 
-fp-model=except but no rounding mode has been requested (again via command 
line options) then the rounding mode should be set to "round to nearest".  I'm 
following a description of how the icc compiler works. I'm afraid that your 
concise comment "eb?" doesn't convey enough information for me to understand 
what you mean.  With these further remarks is it clear now? 



Comment at: llvm/lib/IR/FPState.cpp:78
+  }
+
+  Builder.setIsFPConstrained(IsConstrainedExcept || IsConstrainedRounding);

kpn wrote:
> The IRBuilder already has defaults for exception behavior and rounding. Is it 
> a good idea to duplicate that knowledge here? Worse, what's here is different 
> from what's in the IRBuilder. Why not ask the IRBuilder what its current 
> setting is and use that?
> 
> Would it make sense to have setters/getters, and then have a separate 
> updateBuilder() method? We still don't have a good way to get the #pragma 
> down to the lower levels of clang. The current, unfinished, attempt doesn't 
> work for C++ templates and I'm told other cases as well. An updateBuilder() 
> method could be helpful when moving from one scope to another. But keep in 
> mind that if any constrained fp math is used in a function then the entire 
> function has to be constrained.
> 
> Given that last bit, it may make more sense to have the support somewhere 
> higher level than as a wrapper around the IRBuilder. Maybe in CodeGenFunction 
> or CodeGenModule? I've not spent much time in clang so I'm not sure if that 
> makes sense or not.
Yes I absolutely don't want to duplicate, and I will submit another version of 
this patch and i'll be removing fpstate*.  I do want to be able to make the 
fp-model options match the same behavior as icc is using.  One reason i wanted 
to keep track of the state within a separate object is because i was uncertain 
if stuff would be going on in the IR builder

[PATCH] D65343: [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder created this revision.
tmroeder added a reviewer: Eugene.Zelenko.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

This changes ReleaseNotes.txt to have the first sentence of the full
documentation from linuxkernel-must-use-errs.rst.

This addresses a comment from the review of rL367071 
 in
https://reviews.llvm.org/D59963.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65343

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst


Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions 
that
+directly return a value from one of these error functions.
+
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions 
that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_unused_result))``, but some kernel versions do not have
-  this warning enabled for clang.
+  ``linux/err.h``.
 
 Improvements to include-fixer
 -


Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions that
+directly return a value from one of these error functions.
+
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ER

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64838#1602840 , 
@Nathan-Huckleberry wrote:

> I agree that parsing according to attribute name/type is not a good solution.
>
> It sounds like we have narrowed it down to two choices:
>  Do we want to follow the gcc method of parsing once and falling back if 
> parsing fails?
>  Do we want to parse attributes first and then wait until we see a 
> decl-specifier (breaking the implicit int case)?


I don't think so. A GCC attribute is a decl-specifier, so should trigger 
implicit-int in the languages that have it.

Option 1: teach the statement/declaration disambiguation code that an initial 
GNU attribute does not resolve the ambiguity and that it needs to disambiguate 
past one.

Option 2: parse the attributes and then call the disambiguation code and tell 
it that we've already consumed a decl-specifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder marked an inline comment as done.
tmroeder added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:77
+
+  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return

tmroeder wrote:
> Eugene.Zelenko wrote:
> > tmroeder wrote:
> > > Eugene.Zelenko wrote:
> > > > Release Notes should include short description. One sentence is enough, 
> > > > but it'll good idea to keep it same as first statement of documentation.
> > > Would you like me to submit a patch that removes the second paragraph of 
> > > this description, then?
> > Yes. Documentation is created for details. But you still need to make its 
> > first sentence same as in Release Notes. See other checks and their Release 
> > Notes as example (in previous release branches).
> I'm sorry, I don't understand. What's the referent of "it" in "you still need 
> to make its first sentence same as in Release Notes"? What first sentence 
> needs to match the first sentence of the Release Notes?
> 
> If it's the header file documentation (MustCheckErrs.h), then as far as I can 
> tell, the first paragraph of Release Notes is identical to the first 
> paragraph of the .h file documentation, other than the double backquotes, 
> which I didn't think belonged in a .h file comment.
> 
> What am I missing?
I have looked back, and I think that https://reviews.llvm.org/D65343 should 
address the comment here. Please take a look at let me know.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D65308: [NFC][clang] Refactor getCompilationPhases()+Types.def step 3.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 211981.
plotfi marked an inline comment as done.
plotfi added a comment.

addressing @compnerd 's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65308

Files:
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -18,15 +18,14 @@
 
 struct TypeInfo {
   const char *Name;
-  const char *Flags;
   const char *TempSuffix;
   ID PreprocessedType;
   const llvm::SmallVector Phases;
 };
 
 static const TypeInfo TypeInfos[] = {
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS, ...) \
-  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, { __VA_ARGS__ }, },
+#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, ...) \
+  { NAME, TEMP_SUFFIX, TY_##PP_TYPE, { __VA_ARGS__ }, },
 #include "clang/Driver/Types.def"
 #undef TYPE
 };
@@ -90,7 +89,15 @@
 }
 
 bool types::canTypeBeUserSpecified(ID Id) {
-  return strchr(getInfo(Id).Flags, 'u');
+  static const clang::driver::types::ID kStaticLangageTypes[] = {
+  TY_CUDA_DEVICE,   TY_HIP_DEVICE,TY_PP_CHeader,
+  TY_PP_ObjCHeader, TY_PP_CXXHeader,  TY_ObjCXXHeader,
+  TY_PP_CXXModule,  TY_LTO_IR,TY_LTO_BC,
+  TY_Plist, TY_RewrittenObjC, TY_RewrittenLegacyObjC,
+  TY_Remap, TY_PCH,   TY_Object,
+  TY_Image, TY_dSYM,  TY_Dependencies,
+  TY_CUDA_FATBIN,   TY_HIP_FATBIN};
+  return !llvm::is_contained(kStaticLangageTypes, Id);
 }
 
 bool types::appendSuffixForType(ID Id) {
Index: clang/include/clang/Driver/Types.h
===
--- clang/include/clang/Driver/Types.h
+++ clang/include/clang/Driver/Types.h
@@ -20,7 +20,7 @@
 namespace types {
   enum ID {
 TY_INVALID,
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS, ...) TY_##ID,
+#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, ...) TY_##ID,
 #include "clang/Driver/Types.def"
 #undef TYPE
 TY_LAST
Index: clang/include/clang/Driver/Types.def
===
--- clang/include/clang/Driver/Types.def
+++ clang/include/clang/Driver/Types.def
@@ -29,76 +29,73 @@
 // The fourth value is the suffix to use when creating temporary files
 // of this type, or null if unspecified.
 
-// The fifth value is a string containing option flags. Valid values:
-//  u - The type can be user specified (with -x).
-
-// The sixth value is a variadic list of phases for each type. Eventually the
+// The final value is a variadic list of phases for each type. Eventually the
 // options flag string will be replaced with this variadic list.
 // Most of the options in Flags have been removed in favor of subsuming their
 // meaning from the phases list.
 
 // C family source language (with and without preprocessing).
-TYPE("cpp-output",   PP_C, INVALID, "i", "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("c",C,PP_C,"c", "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cl",   CL,   PP_C,"cl","u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cuda-cpp-output",  PP_CUDA,  INVALID, "cui",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cuda", CUDA, PP_CUDA, "cu","u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cuda", CUDA_DEVICE,  PP_CUDA, "cu","" , phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip-cpp-output",   PP_HIP,   INVALID, "cui",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip",  HIP,  PP_HIP,  "cu","u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip",  HIP_DEVICE,   PP_HIP,  "cu","" , phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objective-c-cpp-output",   PP_ObjC,  INVALID, "mi","u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objc-cpp-output",  PP_ObjC_Alias, INVALID,"mi","u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objective-c",  ObjC, PP_ObjC, "m", "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("c++-cpp-output",   PP_CXX,   INVALID, "ii","u", ph

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 211979.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.

Change attribute used to check special case of global namespace; IsPathValid 
changed to IsInGlobalNamespace. This attribute is true when its first parent is 
the global namespace or if the info is the global namespace,


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

https://reviews.llvm.org/D64958

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -80,7 +80,8 @@
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "path/to/F");
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ ""); // F is in the global namespace
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
@@ -120,7 +121,7 @@
 Parents:
   - Type:Record
 Name:'F'
-Path:'path/to/F'
+IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsInGlobalNamespace", Ref.IsInGlobalNamespace, false);
   }
 };
 
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -114,11 +114,17 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
   Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
-  : USR(USR), Name(Name), RefType(IT), Path(Path) {}
+  : USR(USR), Name(Name), RefType(IT), Path(Path),
+IsInGlobalNamespace(Path.empty()) {}
 
   bool operator==(const Reference &Other) const {
 return std::tie(USR, Name, RefType) ==
@@ -130,8 +136,12 @@
   InfoType RefType = InfoType::IT_default; // Indicates the type of this
// Reference (namespace, record,
// function, enum, default).
-  llvm::SmallString<128> Path; // Path of directory where the clang-doc
-   // generated file will be saved
+  llvm::SmallString<128>
+  Path; // Path of directory where the clang-doc generated file will be
+// saved (possibly unresolved)
+  bool IsInGlobalNamespace =
+  false; // Indicates if the info's parent is the global namespace, or if
+ // the info is the global namespace
 };
 
 // A base struct for TypeInfos
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -245,7 +245,7 @@
 
 static std::unique_ptr genTypeReference(const Reference &Type,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ cl

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Explorers aren't the right abstraction. For the purposes of displaying svg 
files we don't care in which order do we explore the nodes. We may care about 
this for other analyses, but we're not there yet.

The function of cutting out chunks of the graph is performed poorly by the 
explorers, because querying predecessors/successors on the explored nodes 
yields original successors/predecessors even if they aren't being explored.

Introduce a new entity, "trimmers", that do one thing but to it right: cut out 
chunks of the graph. Trimmers mutate the graph, so stale edges aren't even 
visible to their consumers in the pipeline. Additionally, trimmers are 
intrinsically composable: multiple trimmers can be applied to the graph 
sequentially.

Refactor the single-path explorer into the single-path trimmer. Rename the test 
file for consistency.


Repository:
  rC Clang

https://reviews.llvm.org/D65344

Files:
  clang/test/Analysis/exploded-graph-rewriter/explorers.dot
  clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -882,37 +882,36 @@
 visitor.visit_end_of_graph()
 
 
-# SinglePathExplorer traverses only a single path - the leftmost path
-# from the root. Useful when the trimmed graph is still too large
-# due to a large amount of equivalent reports.
-class SinglePathExplorer(object):
-def __init__(self):
-super(SinglePathExplorer, self).__init__()
+#===---===#
+# Trimmers cut out parts of the ExplodedGraph so that to focus on other parts.
+# Trimmers can be combined together by applying them sequentially.
+#===---===#
 
-def explore(self, graph, visitor):
-visitor.visit_begin_graph(graph)
 
-# Keep track of visited nodes in order to avoid loops.
-visited = set()
+# SinglePathTrimmer keeps only a single path - the leftmost path from the root.
+# from the root. Useful when the trimmed graph is still too large.
+class SinglePathTrimmer(object):
+def __init__(self):
+super(SinglePathTrimmer, self).__init__()
+
+def trim(self, graph):
+visited_nodes = set()
 node_id = graph.root_id
 while True:
-visited.add(node_id)
+visited_nodes.add(node_id)
 node = graph.nodes[node_id]
-logging.debug('Visiting ' + node_id)
-visitor.visit_node(node)
-if len(node.successors) == 0:
-break
-
-succ_id = node.successors[0]
-succ = graph.nodes[succ_id]
-logging.debug('Visiting edge: %s -> %s ' % (node_id, succ_id))
-visitor.visit_edge(node, succ)
-if succ_id in visited:
+if len(node.successors) > 0:
+succ_id = node.successors[0]
+succ = graph.nodes[succ_id]
+node.successors = [succ_id]
+succ.predecessors = [node_id]
+if succ_id in visited_nodes:
+break
+node_id = succ_id
+else:
 break
-
-node_id = succ_id
-
-visitor.visit_end_of_graph()
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
 
 
 #===---===#
@@ -960,10 +959,18 @@
 raw_line = raw_line.strip()
 graph.add_raw_line(raw_line)
 
-explorer = SinglePathExplorer() if args.single_path else BasicExplorer()
+trimmers = []
+if args.single_path:
+trimmers.append(SinglePathTrimmer())
+
+explorer = BasicExplorer()
+
 visitor = DotDumpVisitor(args.diff, args.dark, args.gray, args.topology,
  args.rewrite_only)
 
+for trimmer in trimmers:
+trimmer.trim(graph)
+
 explorer.explore(graph, visitor)
 
 


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -882,37 +882,36 @@
 visitor.visit_end_of_graph()
 
 
-# SinglePathExplorer traverses only a single path - the leftmost path
-# from the root. Useful when the trimmed graph is still too large
-# due to a large amount of equivalent reports.
-class SinglePathExplorer(object):
-def __init__(self)

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea marked 2 inline comments as done.
gtbercea added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7695
   Flags &= ~(OMP_MAP_TO | OMP_MAP_FROM | OMP_MAP_ALWAYS |
- OMP_MAP_DELETE);
+ OMP_MAP_DELETE | OMP_MAP_CLOSE);
 

ABataev wrote:
> Why?
If the pointee has been mapped as TO/FROM/etc already no need to map it 
TO/FROM/etc again.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

I actually don't have much of an opinion on what the command line argument form 
should be. It may be helpful for it to be the same as one of the commonly 
deployed compilers. The worst I think would be pretty close but with subtle 
differences. So if it can be made to work like Intel's compiler I'm fine with 
that. But I'm hoping that more people in the community chime in since having a 
consensus would be best. Personally, I'm not yet giving any final sign-offs to 
tickets since I don't think I've been here long enough.

As far as the rounding metadata argument, the language reference says this:

> For values other than “round.dynamic” optimization passes may assume that the 
> actual runtime rounding mode (as defined in a target-specific manner) matches 
> the specified rounding mode, but this is not guaranteed. Using a specific 
> non-dynamic rounding mode which does not match the actual rounding mode at 
> runtime results in undefined behavior.

Be aware that currently neither of the metadata arguments does anything. They 
get dropped when llvm reaches the SelectionDAG. And none of the optimization 
passes that run before that know anything about constrained intrinsics at all. 
This means they treat code that has them conservatively. Preserving and using 
that metadata in the optimization passes and getting it down and used by the MI 
layer is planned but hasn't happened yet. So the full set of arguments may not 
make sense yet, but an on/off switch for strict mode hopefully does.




Comment at: llvm/lib/IR/FPState.cpp:78
+  }
+
+  Builder.setIsFPConstrained(IsConstrainedExcept || IsConstrainedRounding);

mibintc wrote:
> kpn wrote:
> > The IRBuilder already has defaults for exception behavior and rounding. Is 
> > it a good idea to duplicate that knowledge here? Worse, what's here is 
> > different from what's in the IRBuilder. Why not ask the IRBuilder what its 
> > current setting is and use that?
> > 
> > Would it make sense to have setters/getters, and then have a separate 
> > updateBuilder() method? We still don't have a good way to get the #pragma 
> > down to the lower levels of clang. The current, unfinished, attempt doesn't 
> > work for C++ templates and I'm told other cases as well. An updateBuilder() 
> > method could be helpful when moving from one scope to another. But keep in 
> > mind that if any constrained fp math is used in a function then the entire 
> > function has to be constrained.
> > 
> > Given that last bit, it may make more sense to have the support somewhere 
> > higher level than as a wrapper around the IRBuilder. Maybe in 
> > CodeGenFunction or CodeGenModule? I've not spent much time in clang so I'm 
> > not sure if that makes sense or not.
> Yes I absolutely don't want to duplicate, and I will submit another version 
> of this patch and i'll be removing fpstate*.  I do want to be able to make 
> the fp-model options match the same behavior as icc is using.  One reason i 
> wanted to keep track of the state within a separate object is because i was 
> uncertain if stuff would be going on in the IR builder which would be 
> changing the settings, for whatever reason, and i'd want to put them back 
> into settings specified by the command line options before creating the 
> constrained intrinsics in clang/codegen. 
> 
> let me work on this patch more, i just did a quick update to the latest 
> before sending this up.
> 
> As far as pragmas versus templates, this is a concern.  Is there something I 
> can read to learn more about the issue?  Pragma's are used in OpenMP so there 
> must be a way to have pragma's interact politely with templates?  Knowing 
> very little, I thought that the pragma would be held as a _Pragma, sort of 
> like a function call, within the intermediate representation e.g. as opposed 
> to a token stream from the preprocessor and I didn't think there would be a 
> problem with templates per se. I'll check with other folks here at Intel. 
> There was  a concern about inlining constrained intrinsics into a function 
> because of your rule about whole function body but nobody mentioned a problem 
> with templates.
See "https://reviews.llvm.org/D52839";, "Inform AST's UnaryOperator of 
FENV_ACCESS". It was there that Richard Smith brought up the issue of 
templates. I've been prioritizing work on the llvm end and haven't had a chance 
to get to understand how the relevant parts on the clang side work myself. My 
hope is that maybe command line arguments can go in to enable strict FP on a 
compilation-wide basis, with support for the #pragma coming later. But I don't 
know if it will work out that way.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D65308: [NFC][clang] Refactor getCompilationPhases()+Types.def step 3.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65308



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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 211987.
Nathan-Huckleberry added a comment.

- Order swap and elif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454

Files:
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.CallAndMessage.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DivideZero.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.DynamicTypePropagation.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NonNullParamChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.NullDereference.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.StackAddressEscape.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.UndefinedBinaryOperatorResult.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.VLASize.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.ArraySubscript.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Assign.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.Branch.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.CapturedBlockVariable.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-core.uninitialized.UndefReturn.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.InnerPointer.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.Move.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDelete.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-cplusplus.NewDeleteLeaks.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-deadcode.DeadStores.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullPassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableDereferenced.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullablePassedToNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-nullability.NullableReturnedFromNonnull.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.UninitializedObject.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.cplusplus.VirtualCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.mpi.MPI-Checker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.OSObjectCStyleCast.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.GCDAntipattern.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.performance.Padding.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-optin.portability.UnixAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.API.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.MIG.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.NumberObjectConversion.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.OSObjectRetainCount.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.ObjCProperty.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.SecKeychainAPI.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AtSync.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.AutoreleaseWrite.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ClassRelease.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Dealloc.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.IncompatibleMethodTypes.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.Loops.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.MissingSuperCall.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSAutoreleasePool.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NSError.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NilArg.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.NonNilReturnValue.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.ObjCGenerics.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RetainCount.rst
  
clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.rst
  clang-tools-extra/docs/clang-tidy/checks/clang-analyzer-osx.cocoa.SelfInit.rst
  
clang-tools-ex

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

[Emmett Neyman via Phabricator via cfe-commits]

emmettneyman marked 16 inline comments as done.
emmettneyman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1948
+def RequiresDesignator : InheritableAttr {
+  let Spellings = [Clang<"requires_designator">, GCC<"designated_init">];
+  let Subjects = SubjectList<[Record]>;

aaron.ballman wrote:
> Why does this have a `Clang` spelling in addition to the `GCC` spelling? I 
> think it only needs the `GCC` spelling.
I kept the `Clang` spelling so that there's naming consistency between the two 
attributes (`requires_designator` and `requires_init`). I can remove it if it's 
redundant/unnecessary, let me know.



Comment at: clang/include/clang/Basic/AttrDocs.td:1484-1485
+requires designated initializers be used rather than positional initializers.
+This attribute additionally has a stronger restriction that declarations must 
be
+brace-initialized (which is why ``Foo foo;`` is considered invalid above. The
+final way this attribute differs from GCC's ``designated_init`` attribute is

aaron.ballman wrote:
> Why is it valuable to differ from GCC's behavior in this regard?
The motivation behind GCC's attribute seems to be to avoid using positional 
arguments:
> The intent of this attribute is to allow the programmer to indicate that a 
> structure’s layout may change, and that therefore relying on positional 
> initialization will result in future breakage.

The motivation behind this attribute is a little bit different. Instead of 
avoiding the use of positional arguments, the goal of this attribute is to 
bring ObjC-style named parameters to C/C++ and to be able to enforce their use 
in certain situations. In line with this goal, we wanted to forbid the 
following pattern:
```
Foo foo;
foo.x = 42;
foo.y = 36;
```
That's why this attribute enforces that all declarations be brace-initialized.



Comment at: clang/include/clang/Basic/AttrDocs.td:1487
+final way this attribute differs from GCC's ``designated_init`` attribute is
+that it can be applied to union and class types, as well as struct types.
+  }];

aaron.ballman wrote:
> Given that it can be added to class types, I wonder what the behavior should 
> be for code like:
> ```
> struct Foo {
>   int a = 1;
>   int b, c;
>   int d = 4;
> };
> 
> Foo foo = {...};
> ```
> Does the initializer for `foo` have to specify `.a` and `.d`?
The `requires_designator` attribute doesn't require that any of the fields have 
to be specified, only that designated initializer syntax has to be used. So, 
for the struct definition above, any of the following are valid:
```
Foo foo {};
Foo foo {.a = 1, .b = 2, .c = 3, .d = 4};
Foo foo {.b = 2, .c = 3};
```
Any of the fields can be left out and default-initialized. I've added this 
example to the lit tests for the attribute.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3537
+def note_declared_requires_designator_here : Note<
+  "required by 'requires_designator' attribute here">;
+def warn_requires_init_failed : Warning<

aaron.ballman wrote:
> This attribute spelling seems wrong, it should be `designated_init`, no?
Does it make more sense to use `designated_init` or `requires_designator` as 
the primary spelling for this attribute? I chose to use the 
`requires_designator` spelling so that the two attributes have consistent and 
similar spellings. Let me know if you think it should be the other way around 
(or if you want the `Clang` spelling removed entirely).



Comment at: clang/lib/Sema/SemaDecl.cpp:11216
+// If the type of the declaration is a struct/class and that type has the
+// require_designated_init attribute, check that the initializer has
+// the proper designated initializer syntax.

aaron.ballman wrote:
> Attribute spelling is stale in this comment.
See comments above about keeping this spelling.



Comment at: clang/lib/Sema/SemaDecl.cpp:15314
 
+  // If this TagDecl has any non-public fields, all requires_designator and
+  // requires_init attributes should be ignored.

aaron.ballman wrote:
> Attribute name is stale in this comment.
> 
> This is the wrong place to do this work -- it should be done from 
> SemaDeclAttr.cpp when processing the attribute. We should avoid adding the 
> attribute in the first place rather than adding the attribute and then later 
> removing it.
See question above regarding attribute spelling.

I thought about this a lot because I agree, it feels wrong to remove the 
attribute once it is already added to a field, struct, etc. However I could not 
think of any other way or any other place to do this check. Since all fields of 
the struct need to have been processed already in order to check whether there 
are any non-public members, it seems like the only place to do this check is at 
the end of handling the entire `TagDecl`. When the struct-

[PATCH] D65345: [analyzer] exploded-graph-rewriter: Implement manual graph trimming.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ added a parent revision: D65344: [analyzer] exploded-graph-rewriter: NFC: 
Replace explorers with trimmers..

When `-trim-egraph` is unavailable (say, when you're debugging a crash on a 
real-world code that takes too long to reduce), it makes sense to view the 
untrimmed graph up to the crashing node's predecessor, then dump the ID (or a 
pointer) of the node in the attached debugger, and then trim the dumped graph 
in order to keep only paths from the root to the node.

Implement the last part of that plan. Now you can do:

  $ exploded-graph-rewriter.py ExprEngine.dot --to 0x12229acd0

And it'll chop out the unnecessary chunks of the graph. You can also specify 
multiple nodes and you can specify nodes by stable IDs rather than by pointers.


Repository:
  rC Clang

https://reviews.llvm.org/D65345

Files:
  clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -914,6 +914,52 @@
for node_id in visited_nodes}
 
 
+# TargetedTrimmer keeps paths that lead to specific nodes and discards all
+# other paths. Useful when you cannot use -trim-egraph (eg., when debugging
+# a crash).
+class TargetedTrimmer(object):
+def __init__(self, target_nodes):
+super(TargetedTrimmer, self).__init__()
+self._target_nodes = target_nodes
+
+@staticmethod
+def parse_target_node(node, graph):
+if node.startswith('0x'):
+ret = 'Node' + node
+assert ret in graph.nodes
+return ret
+else:
+for other_id in graph.nodes:
+other = graph.nodes[other_id]
+if other.node_id == int(node):
+return other_id
+
+@staticmethod
+def parse_target_nodes(target_nodes, graph):
+return [TargetedTrimmer.parse_target_node(node, graph)
+for node in target_nodes.split(',')]
+
+def trim(self, graph):
+queue = self._target_nodes
+visited_nodes = set()
+
+while len(queue) > 0:
+node_id = queue.pop()
+visited_nodes.add(node_id)
+node = graph.nodes[node_id]
+for pred_id in node.predecessors:
+if pred_id not in visited_nodes:
+queue.append(pred_id)
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
+for node_id in graph.nodes:
+node = graph.nodes[node_id]
+node.successors = [succ_id for succ_id in node.successors
+   if succ_id in visited_nodes]
+node.predecessors = [succ_id for succ_id in node.predecessors
+ if succ_id in visited_nodes]
+
+
 #===---===#
 # The entry point to the script.
 #===---===#
@@ -939,6 +985,11 @@
 help='only display the leftmost path in the graph '
  '(useful for trimmed graphs that still '
  'branch too much)')
+parser.add_argument('--to', type=str, default=None,
+help='display only execution paths from the root '
+ 'to the given comma-separated list of nodes '
+ 'identified by a pointer or a stable ID; '
+ 'compatible with --single-path')
 parser.add_argument('--dark', action='store_const', dest='dark',
 const=True, default=False,
 help='dark mode')
@@ -960,6 +1011,9 @@
 graph.add_raw_line(raw_line)
 
 trimmers = []
+if args.to is not None:
+trimmers.append(TargetedTrimmer(
+TargetedTrimmer.parse_target_nodes(args.to, graph)))
 if args.single_path:
 trimmers.append(SinglePathTrimmer())
 
Index: clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
+++ clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
@@ -1,7 +1,17 @@
 // RUN: %exploded_graph_rewriter %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
 // RUN: %exploded_graph_rewriter -s %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
+// R

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

[Emmett Neyman via Phabricator via cfe-commits]

emmettneyman updated this revision to Diff 211988.
emmettneyman marked 6 inline comments as done.
emmettneyman added a comment.

Addressed comments and feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64380

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-requires-designator.cpp
  clang/test/SemaCXX/attr-requires-init.cpp

Index: clang/test/SemaCXX/attr-requires-init.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-requires-init.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+[[clang::requires_init]] int x;// expected-warning{{'requires_init' attribute only applies to non-static data members}}
+[[clang::requires_init]] void fun(int x) { // expected-warning{{'requires_init' attribute only applies to non-static data members}}
+  return;
+}
+struct [[clang::requires_init]] Foo { // expected-warning{{'requires_init' attribute only applies to non-static data members}}
+  int x;
+};
+
+// Struct with one required field
+struct Bar {
+  [[clang::requires_init]] int y; // expected-note 0+ {{enforced by 'requires_init' attribute here}}
+};
+
+// The following are invalid ways of initializing instances of this struct.
+Bar b1;// expected-warning{{initializer for variable b1 must explicitly initialize field y}}
+Bar b2{};  // expected-warning{{initializer for variable b2 must explicitly initialize field y}}
+Bar b3{1}; // expected-warning{{initializer for variable b3 must explicitly initialize field y}}
+
+// The following are valid ways of initializing instances of this struct.
+Bar b6{.y = 1};
+
+// Struct with multiple required fields
+struct Baz {
+  [[clang::requires_init]] int x; // expected-note 0+ {{enforced by 'requires_init' attribute here}}
+  int y;
+  [[clang::requires_init]] int z; // expected-note 0+ {{enforced by 'requires_init' attribute here}}
+};
+
+// The following are invalid ways of initializing instances of this struct.
+Baz z1; // expected-warning{{initializer for variable z1 must explicitly initialize field x}} expected-warning{{initializer for variable z1 must explicitly initialize field z}}
+Baz z2{};   // expected-warning{{initializer for variable z2 must explicitly initialize field x}} expected-warning{{initializer for variable z2 must explicitly initialize field z}}
+Baz z3{1, 2};   // expected-warning{{initializer for variable z3 must explicitly initialize field x}} expected-warning{{initializer for variable z3 must explicitly initialize field z}}
+Baz z4{1, 2, 3};// expected-warning{{initializer for variable z4 must explicitly initialize field x}} expected-warning{{initializer for variable z4 must explicitly initialize field z}}
+Baz z5{.x = 1, 2};  // expected-warning{{initializer for variable z5 must explicitly initialize field z}}
+Baz z6{.x = 1, .y = 2}; // expected-warning{{initializer for variable z6 must explicitly initialize field z}}
+
+// The following are valid ways of initializing instances of this struct.
+Baz z7{.x = 1, .y = 2, .z = 3};
+Baz z8{.x = 1, .z = 3};
+Baz z9{.x = 1, 2, .z = 3};
+
+// The requires_init attribute can also be applied to public fields of classes.
+class Cla {
+public:
+  [[clang::requires_init]] int x; // expected-note 0+ {{enforced by 'requires_init' attribute here}}
+  int y;
+};
+
+// The following are invalid ways of initializing instances of this class.
+Cla c1;// expected-warning{{initializer for variable c1 must explicitly initialize field x}}
+Cla c2{};  // expected-warning{{initializer for variable c2 must explicitly initialize field x}}
+Cla c3{1}; // expected-warning{{initializer for variable c3 must explicitly initialize field x}}
+Cla c4{1, 2};  // expected-warning{{initializer for variable c4 must explicitly initialize field x}}
+Cla c5{1, .y = 2}; // expected-warning{{initializer for variable c5 must explicitly initialize field x}}
+
+// The following are valid ways of initializing instances of this class.
+Cla c6{.x = 1};
+Cla c7{.x = 1, .y = 2};
+Cla c8{.x = 1, 2};
+
+// This attribute cannot be applied to fields of a union.
+union Uni {
+  [[clang::requires_init]] int x; // expected-warning{{'requires_init' attribute ignored}}
+  int y;
+};
+
+// If any of the fields in the record are non-public all requires_init
+// attributes in the record are ignored.
+struct PriMems {
+  [[clang::requires_init]] int x; // expected-warning{{'requires_init' attribute ignored}}
+private:
+  int y;
+};
+PriMems pm1;
+PriMems pm2();
+
+class PriClass {
+  int x;
+public:
+  [[clang::requires_in

[PATCH] D65270: [CMake] Fix source path generation for install in multi-config (MSBuild)

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

LGTM, @beanz and @smeenai do you have any further comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65270



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 211991.
lildmh added a comment.

Make `emitUDMapperArrayInitOrDel` private


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

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,414 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-tar

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

[Csaba Dabis via Phabricator via cfe-commits]

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

In my mind an explorer would trim me a graph so I like the new abstraction to 
differentiate the two. Thanks!




Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:892
+# SinglePathTrimmer keeps only a single path - the leftmost path from the root.
+# from the root. Useful when the trimmed graph is still too large.
+class SinglePathTrimmer(object):

`from the root.` duplicated from the above line.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65344



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

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 211992.
NoQ added a comment.

Fxd.


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

https://reviews.llvm.org/D65344

Files:
  clang/test/Analysis/exploded-graph-rewriter/explorers.dot
  clang/test/Analysis/exploded-graph-rewriter/trimmers.dot
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -882,37 +882,36 @@
 visitor.visit_end_of_graph()
 
 
-# SinglePathExplorer traverses only a single path - the leftmost path
-# from the root. Useful when the trimmed graph is still too large
-# due to a large amount of equivalent reports.
-class SinglePathExplorer(object):
-def __init__(self):
-super(SinglePathExplorer, self).__init__()
+#===---===#
+# Trimmers cut out parts of the ExplodedGraph so that to focus on other parts.
+# Trimmers can be combined together by applying them sequentially.
+#===---===#
 
-def explore(self, graph, visitor):
-visitor.visit_begin_graph(graph)
 
-# Keep track of visited nodes in order to avoid loops.
-visited = set()
+# SinglePathTrimmer keeps only a single path - the leftmost path from the root.
+# Useful when the trimmed graph is still too large.
+class SinglePathTrimmer(object):
+def __init__(self):
+super(SinglePathTrimmer, self).__init__()
+
+def trim(self, graph):
+visited_nodes = set()
 node_id = graph.root_id
 while True:
-visited.add(node_id)
+visited_nodes.add(node_id)
 node = graph.nodes[node_id]
-logging.debug('Visiting ' + node_id)
-visitor.visit_node(node)
-if len(node.successors) == 0:
-break
-
-succ_id = node.successors[0]
-succ = graph.nodes[succ_id]
-logging.debug('Visiting edge: %s -> %s ' % (node_id, succ_id))
-visitor.visit_edge(node, succ)
-if succ_id in visited:
+if len(node.successors) > 0:
+succ_id = node.successors[0]
+succ = graph.nodes[succ_id]
+node.successors = [succ_id]
+succ.predecessors = [node_id]
+if succ_id in visited_nodes:
+break
+node_id = succ_id
+else:
 break
-
-node_id = succ_id
-
-visitor.visit_end_of_graph()
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
 
 
 #===---===#
@@ -960,10 +959,18 @@
 raw_line = raw_line.strip()
 graph.add_raw_line(raw_line)
 
-explorer = SinglePathExplorer() if args.single_path else BasicExplorer()
+trimmers = []
+if args.single_path:
+trimmers.append(SinglePathTrimmer())
+
+explorer = BasicExplorer()
+
 visitor = DotDumpVisitor(args.diff, args.dark, args.gray, args.topology,
  args.rewrite_only)
 
+for trimmer in trimmers:
+trimmer.trim(graph)
+
 explorer.explore(graph, visitor)
 
 


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -882,37 +882,36 @@
 visitor.visit_end_of_graph()
 
 
-# SinglePathExplorer traverses only a single path - the leftmost path
-# from the root. Useful when the trimmed graph is still too large
-# due to a large amount of equivalent reports.
-class SinglePathExplorer(object):
-def __init__(self):
-super(SinglePathExplorer, self).__init__()
+#===---===#
+# Trimmers cut out parts of the ExplodedGraph so that to focus on other parts.
+# Trimmers can be combined together by applying them sequentially.
+#===---===#
 
-def explore(self, graph, visitor):
-visitor.visit_begin_graph(graph)
 
-# Keep track of visited nodes in order to avoid loops.
-visited = set()
+# SinglePathTrimmer keeps only a single path - the leftmost path from the root.
+# Useful when the trimmed graph is still too large.
+class SinglePathTrimmer(object):
+def __init__(self):
+super(SinglePathTrimmer, self).__init__()
+
+def trim(self, graph):
+visited_nodes = set()
 node_id = graph.root_id
 while True:
-visited.add(node_id)
+visited_nodes.add(node_id)
 n

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

[Csaba Dabis via Phabricator via cfe-commits]

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

I like the HTML output, thanks!




Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:846
+print('  $ dot -Tsvg input.dot -o output.svg')
+print()
+write_temp_file('.dot', self.output())

Why we need multiple prints to print one message? According to the Stack 
Overflow:
```
print("""
Line1
Line2
""")
```

Link: 
https://stackoverflow.com/questions/34980251/how-to-print-multiple-lines-of-text-with-python



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:953
+ 'displaying it, dump the rewritten dot file '
+ 'to stdout')
 args = parser.parse_args()

This flag is a little-bit too long and does not really emphasize what it 
suppose to do. What about just `--dump` or `--stdout`?


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

https://reviews.llvm.org/D65250



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

[PATCH] D65343: [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Looks OK, but please make this changes in D59963 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65343



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

r367147 - Partially revert rC365414; `ln -n` is not portable

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Fri Jul 26 13:09:37 2019
New Revision: 367147

URL: http://llvm.org/viewvc/llvm-project?rev=367147&view=rev
Log:
Partially revert rC365414; `ln -n` is not portable

This restores the use of `rm` instead of the non-portable `ln -n`. Such
use being the status quo for the 12-month period between rC334972 and
rC365414.

Modified:
cfe/trunk/test/Driver/no-canonical-prefixes.c

Modified: cfe/trunk/test/Driver/no-canonical-prefixes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/no-canonical-prefixes.c?rev=367147&r1=367146&r2=367147&view=diff
==
--- cfe/trunk/test/Driver/no-canonical-prefixes.c (original)
+++ cfe/trunk/test/Driver/no-canonical-prefixes.c Fri Jul 26 13:09:37 2019
@@ -4,10 +4,11 @@
 // RUN: cd %t.real
 // RUN: ln -sf %clang test-clang
 // RUN: cd ..
-// If %.fake already is a symlink to %t.real when `ln -sf %t.real %t.fake`
-// runs, then that would symlink %t.real to itself, forming a cycle.
-// The `-n` flag prevents this.
-// RUN: ln -sfn %t.real %t.fake
+// Important to remove %t.fake: If it already is a symlink to %t.real when
+// `ln -sf %t.real %t.fake` runs, then that would symlink %t.real to itself,
+// forming a cycle.
+// RUN: rm -f %t.fake
+// RUN: ln -sf %t.real %t.fake
 // RUN: cd %t.fake
 // RUN: ./test-clang -v -S %s 2>&1 | FileCheck --check-prefix=CANONICAL %s
 // RUN: ./test-clang -v -S %s -no-canonical-prefixes 2>&1 | FileCheck 
--check-prefix=NON-CANONICAL %s


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

[PATCH] D65343: [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Sorry, I didn't notice that D59963  was 
committed. In general it's not good idea to commit changes when patch was not 
approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65343



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