r338230 - PR38355 Prevent infinite recursion when checking initializer lifetime if

2018-07-30 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Mon Jul 30 00:19:54 2018
New Revision: 338230

URL: http://llvm.org/viewvc/llvm-project?rev=338230&view=rev
Log:
PR38355 Prevent infinite recursion when checking initializer lifetime if
an initializer is self-referential.

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/SemaCXX/warn-dangling-local.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=338230&r1=338229&r2=338230&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Jul 30 00:19:54 2018
@@ -6570,7 +6570,8 @@ static void visitLocalsRetainedByInitial
   [&](IndirectLocalPath &Path, Local L, ReferenceKind RK) -> bool {
 if (auto *DRE = dyn_cast(L)) {
   auto *VD = dyn_cast(DRE->getDecl());
-  if (VD && VD->getType().isConstQualified() && VD->getInit()) {
+  if (VD && VD->getType().isConstQualified() && VD->getInit() &&
+  !isVarOnPath(Path, VD)) {
 Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD});
 visitLocalsRetainedByInitializer(Path, VD->getInit(), Visit, true);
   }

Modified: cfe/trunk/test/SemaCXX/warn-dangling-local.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-dangling-local.cpp?rev=338230&r1=338229&r2=338230&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-dangling-local.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-dangling-local.cpp Mon Jul 30 00:19:54 2018
@@ -18,3 +18,9 @@ void f() {
   // points to, which doesn't live long enough.
   int *const &s = (int *const &)T{1, 2, 3}; // expected-warning {{temporary 
bound to local reference 's' will be destroyed at the end of the 
full-expression}}
 }
+
+// PR38355
+void g() {
+  const int a[] = {a[0]};
+  const int b[] = {a[0]};
+}


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


[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

2018-07-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.h:106
   CachingCompilationDb CDB;
+  InMemoryCompilationDb InMemoryCDB;
 

This code starts to be a little hard to follow. Could we extract it into an 
external class that encapsulates this logic? Something like:

```
class ClangdLSPServer {

private:
  class CompilationDB {
  public:
static CompilationDB makeInMemory();
stiatc CompilationDB makeDirectoryBased();
  
void invalidate(PathRef File);
 /// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB.
void setCompileCommandsForFile(PathRef File);
/// Only valid for directory-based CDB, no-op and error log on InMemoryCDB;
void setExtraFlags(PathRef File);
/// Returns a CDB that should be used to get compile commands for the 
current instance of ClangdLSPServer.
GlobalCompilationDatabase& getCDB(); 
  
  private:
 // if IsDirectoryBased is true, an instance of InMemoryCDB.
 // If IsDirectoryBased is false, an instance of DirectoryBasedCDB.  
unique_ptr CDB;
unique_ptr CDB;
// non-null only for directory-based CDB
unique_ptr CachingCDB;
bool IsDirectoryBased;
  };
 
  CompilationDB CDB;
  // .
}
```

We can static_cast to InMemoryCDB or DirectoryBasedCDB based on the 
IsDirectoryBased flag to implement all the operations we define in the helper 
class.



Comment at: clangd/GlobalCompilationDatabase.h:136
+  mutable std::mutex Mutex;
+  mutable llvm::StringMap>
+  Commands; /* GUARDED_BY(Mut) */

Maybe remove 'mutable' from Commands? We shouldn't need it.



Comment at: clangd/GlobalCompilationDatabase.h:136
+  mutable std::mutex Mutex;
+  mutable llvm::StringMap>
+  Commands; /* GUARDED_BY(Mut) */

ilya-biryukov wrote:
> Maybe remove 'mutable' from Commands? We shouldn't need it.
Maybe make the value type `CompileCommand` instead of 
`Optional`? 
We seem to only use the optional when inserting values and we could easily 
rewrite that code to use `map::insert()`.



Comment at: clangd/Protocol.h:425
+/// compilation database.
+struct ClangdConfigurationCompilationDatabaseUpdate {
+  std::string workingDirectory;

Maybe use a shorter name, e.g. `ClangdCompileCommand`? This class looks 
*almost* like `tooling::CompileCommand`, so having a similar name seems 
reasonable.




Comment at: clangd/tool/ClangdMain.cpp:170
+static llvm::cl::opt
+InMemoryCompileCommands("in-memory-compile-commands",
+llvm::cl::desc("Use an in-memory compile 
commands"),

The important distinction seems to be where the compile commands come from, 
maybe make the name centered on the fact that compile commands are read from 
the protocol rather from the filesystem?

E.g. using named option values:
`--compile_args_from={lsp|filesystem}`
Or a boolean option:
`--compile_args_from_lsp`

WDYT?



Comment at: clangd/tool/ClangdMain.cpp:171
+InMemoryCompileCommands("in-memory-compile-commands",
+llvm::cl::desc("Use an in-memory compile 
commands"),
+llvm::cl::init(false), llvm::cl::Hidden);

Maybe elaborate a bit more on this mode? E.g. `clangd will get compile all 
commands from via LSP and won't look at the filesystem for 
'compile_commands.json' files`


https://reviews.llvm.org/D49758



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


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

2018-07-30 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 added inline comments.



Comment at: lib/Sema/SemaLambda.cpp:1447
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{LSI.CallOperator, 0.0});
+  }

efriedma wrote:
> This seems sort of late?  You're starting the timer after the body has 
> already been parsed.
Yes and no but if we decided to extend usage of RAII objects - it deos not 
matter. I'll change it.



Comment at: lib/Sema/TreeTransform.h:11011
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }

efriedma wrote:
> What happens if we never hit ActOnFinishFunctionBody()?  TransformLambdaExpr 
> has an early return if the body doesn't typecheck.
> 
> More generally, given that we have early returns all over the place in Sema, 
> I would be more comfortable using the RAII helper, rather than explicitly 
> calling start/stop, even if that means you have to insert FrontendTimeRAII 
> variables in half a dozen different places in the parser.  (Note I'm 
> specifically talking about the parser here; the explicit stopFrontendTimer in 
> ~CodeGenFunction seems fine.)
It seems you're right: usage of RAII objects is safely. OK, I'll change it.


https://reviews.llvm.org/D47196



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


[PATCH] D49862: [clang-tidy] Fix a crash in fuchsia-multiple-inheritance

2018-07-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Sorry for sending this in instead of waiting for 
https://reviews.llvm.org/D49158 to land, I hadn't noticed it earlier


Repository:
  rL LLVM

https://reviews.llvm.org/D49862



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


[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

2018-07-30 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In https://reviews.llvm.org/D49840#1176405, @klimek wrote:

> Usually we use match(anyOf(node), hasDescendant(node)). Or did I 
> misunderstand what you want?


See http://lists.llvm.org/pipermail/cfe-dev/2018-July/058625.html for a bug 
that prevents this working.


Repository:
  rC Clang

https://reviews.llvm.org/D49840



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


[PATCH] D49797: [clang-format] Indent after breaking Javadoc annotated line

2018-07-30 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir updated this revision to Diff 157925.
krasimir added a comment.

- Address comments


Repository:
  rC Clang

https://reviews.llvm.org/D49797

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  unittests/Format/FormatTestComments.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -191,28 +191,38 @@
 
   // Break a single line long jsdoc comment pragma.
   EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
+" * @returns\n"
+" * {string}\n"
+" * jsdoc line 12\n"
 " */",
 format("/** @returns {string} jsdoc line 12 */",
getGoogleJSStyleWithColumns(20)));
 
   EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
+" * @returns\n"
+" * {string}\n"
+" * jsdoc line 12\n"
 " */",
 format("/** @returns {string} jsdoc line 12  */",
getGoogleJSStyleWithColumns(20)));
 
+  // FIXME: this overcounts the */ as a continuation of the 12 when breaking.
+  // Related to the FIXME in BreakableBlockComment::getRangeLength.
   EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
+" * @returns\n"
+" * {string}\n"
+" * jsdoc line\n"
+" * 12\n"
 " */",
 format("/** @returns {string} jsdoc line 12*/",
getGoogleJSStyleWithColumns(20)));
 
   // Fix a multiline jsdoc comment ending in a comment pragma.
   EXPECT_EQ("/**\n"
 " * line 1\n"
 " * line 2\n"
-" * @returns {string} jsdoc line 12\n"
+" * @returns {string}\n"
+" * jsdoc line 12\n"
 " */",
 format("/** line 1\n"
" * line 2\n"
@@ -2028,21 +2038,24 @@
 
 TEST_F(FormatTestJS, JSDocAnnotations) {
   verifyFormat("/**\n"
-   " * @export {this.is.a.long.path.to.a.Type}\n"
+   " * @exports\n"
+   " * {this.is.a.long.path.to.a.Type}\n"
" */",
"/**\n"
-   " * @export {this.is.a.long.path.to.a.Type}\n"
+   " * @exports {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
-   " * @mods {this.is.a.long.path.to.a.Type}\n"
+   " * @mods\n"
+   " * {this.is.a.long.path.to.a.Type}\n"
" */",
"/**\n"
" * @mods {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
   verifyFormat("/**\n"
-   " * @param {this.is.a.long.path.to.a.Type}\n"
+   " * @param\n"
+   " * {this.is.a.long.path.to.a.Type}\n"
" */",
"/**\n"
" * @param {this.is.a.long.path.to.a.Type}\n"
@@ -2058,34 +2071,36 @@
   verifyFormat(
   "/**\n"
   " * @param This is a\n"
-  " * long comment but\n"
-  " * no type\n"
+  " * long comment\n"
+  " * but no type\n"
   " */",
   "/**\n"
   " * @param This is a long comment but no type\n"
   " */",
   getGoogleJSStyleWithColumns(20));
-  // Don't break @param line, but reindent it and reflow unrelated lines.
-  verifyFormat("{\n"
-   "  /**\n"
-   "   * long long long\n"
-   "   * long\n"
-   "   * @param {this.is.a.long.path.to.a.Type} a\n"
-   "   * long long long\n"
-   "   * long long\n"
-   "   */\n"
-   "  function f(a) {}\n"
-   "}",
-   "{\n"
-   "/**\n"
-   " * long long long long\n"
-   " * @param {this.is.a.long.path.to.a.Type} a\n"
-   " * long long long long\n"
-   " * long\n"
-   " */\n"
-   "  function f(a) {}\n"
-   "}",
-   getGoogleJSStyleWithColumns(20));
+  // Break and reindent @param line and reflow unrelated lines.
+  EXPECT_EQ("{\n"
+"  /**\n"
+"   * long long long\n"
+"   * long\n"
+"   * @param\n"
+"   * {this.is.a.long.path.to.a.Type}\n"
+"   * a\n"
+"   * long long long\n"
+"   * long long\n"
+"   */\n"
+"  function f(a) {}\n"
+"}",
+format("{\n"
+   "/**\n"
+   " * long long long long\n"
+   " * @param {this.is.a.long.path.to.a.Type} a\n"
+   " * long long long long\n"
+

Re: [PATCH] D49840: [AST] Add MatchFinder::matchSubtree

2018-07-30 Thread Manuel Klimek via cfe-commits
On Mon, Jul 30, 2018 at 10:02 AM Stephen Kelly via Phabricator <
revi...@reviews.llvm.org> wrote:

> steveire added a comment.
>
> In https://reviews.llvm.org/D49840#1176405, @klimek wrote:
>
> > Usually we use match(anyOf(node), hasDescendant(node)). Or did I
> misunderstand what you want?
>
>
> See http://lists.llvm.org/pipermail/cfe-dev/2018-July/058625.html for a
> bug that prevents this working.
>

Answered on the bug. I don't think that applies here.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r338232 - [clang-format] Indent after breaking Javadoc annotated line

2018-07-30 Thread Krasimir Georgiev via cfe-commits
Author: krasimir
Date: Mon Jul 30 01:45:45 2018
New Revision: 338232

URL: http://llvm.org/viewvc/llvm-project?rev=338232&view=rev
Log:
[clang-format] Indent after breaking Javadoc annotated line

Summary:
This patch makes clang-format indent the subsequent lines created by breaking a
long javadoc annotated line.

Reviewers: mprobst

Reviewed By: mprobst

Subscribers: acoomans, cfe-commits

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

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

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=338232&r1=338231&r2=338232&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Mon Jul 30 01:45:45 2018
@@ -235,6 +235,7 @@ BreakableToken::Split BreakableStringLit
 
 void BreakableStringLiteral::insertBreak(unsigned LineIndex,
  unsigned TailOffset, Split Split,
+ unsigned ContentIndent,
  WhitespaceManager &Whitespaces) const 
{
   Whitespaces.replaceWhitespaceInToken(
   Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
@@ -510,8 +511,33 @@ unsigned BreakableBlockComment::getConte
   return std::max(0, ContentColumn[LineIndex]);
 }
 
+const llvm::StringSet<>
+BreakableBlockComment::ContentIndentingJavadocAnnotations = {
+"@param", "@return", "@returns", "@throws",  "@type", "@template",
+"@see",   "@deprecated", "@define",  "@exports", "@mods",
+};
+
+unsigned BreakableBlockComment::getContentIndent(unsigned LineIndex) const {
+  if (Style.Language != FormatStyle::LK_Java &&
+  Style.Language != FormatStyle::LK_JavaScript)
+return 0;
+  // The content at LineIndex 0 of a comment like:
+  // /** line 0 */
+  // is "* line 0", so we need to skip over the decoration in that case.
+  StringRef ContentWithNoDecoration = Content[LineIndex];
+  if (LineIndex == 0 && ContentWithNoDecoration.startswith("*")) {
+ContentWithNoDecoration = ContentWithNoDecoration.substr(1).ltrim(Blanks);
+  }
+  StringRef FirstWord = ContentWithNoDecoration.substr(
+  0, ContentWithNoDecoration.find_first_of(Blanks));
+  if (ContentIndentingJavadocAnnotations.find(FirstWord) !=
+  ContentIndentingJavadocAnnotations.end())
+return Style.ContinuationIndentWidth;
+  return 0;
+}
+
 void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned 
TailOffset,
-Split Split,
+Split Split, unsigned ContentIndent,
 WhitespaceManager &Whitespaces) const {
   StringRef Text = Content[LineIndex].substr(TailOffset);
   StringRef Prefix = Decoration;
@@ -532,10 +558,14 @@ void BreakableBlockComment::insertBreak(
   Text.data() - tokenAt(LineIndex).TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   assert(LocalIndentAtLineBreak >= Prefix.size());
+  std::string PrefixWithTrailingIndent = Prefix;
+  for (unsigned I = 0; I < ContentIndent; ++I)
+PrefixWithTrailingIndent += " ";
   Whitespaces.replaceWhitespaceInToken(
-  tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "", Prefix,
-  InPPDirective, /*Newlines=*/1,
-  /*Spaces=*/LocalIndentAtLineBreak - Prefix.size());
+  tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "",
+  PrefixWithTrailingIndent, InPPDirective, /*Newlines=*/1,
+  /*Spaces=*/LocalIndentAtLineBreak + ContentIndent -
+  PrefixWithTrailingIndent.size());
 }
 
 BreakableToken::Split
@@ -543,8 +573,17 @@ BreakableBlockComment::getReflowSplit(un
   llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
 return Split(StringRef::npos, 0);
-
+  
+  // If we're reflowing into a line with content indent, only reflow the next
+  // line if its starting whitespace matches the content indent.
   size_t Trimmed = Content[LineIndex].find_first_not_of(Blanks);
+  if (LineIndex) {
+unsigned PreviousContentIndent = getContentIndent(LineIndex - 1);
+if (PreviousContentIndent && Trimmed != StringRef::npos &&
+Trimmed != PreviousContentIndent)
+  return Split(StringRef::npos, 0);
+  }
+
   return Split(0, Trimmed != StringRef::npos ? Trimmed : 0);
 }
 
@@ -583,7 +622,8 @@ void BreakableBlockComment::adaptStartOf
   // break length are the same.
   size_t BreakLength = Lines[0].substr(1).find_first_not_of(Blanks);
   if (BreakLength != StringRef::npos)
-inser

[PATCH] D49797: [clang-format] Indent after breaking Javadoc annotated line

2018-07-30 Thread Krasimir Georgiev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338232: [clang-format] Indent after breaking Javadoc 
annotated line (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D49797

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/BreakableToken.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/unittests/Format/FormatTestComments.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp

Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1809,6 +1809,7 @@
   if (!DryRun)
 Token->adaptStartOfLine(0, Whitespaces);
 
+  unsigned ContentIndent = 0;
   unsigned Penalty = 0;
   LLVM_DEBUG(llvm::dbgs() << "Breaking protruding token at column "
   << StartColumn << ".\n");
@@ -1930,11 +1931,28 @@
 }
   }
   LLVM_DEBUG(llvm::dbgs() << "Breaking...\n");
-  ContentStartColumn =
-  Token->getContentStartColumn(LineIndex, /*Break=*/true);
+  // Update the ContentIndent only if the current line was not reflown with
+  // the previous line, since in that case the previous line should still
+  // determine the ContentIndent. Also never intent the last line.
+  if (!Reflow)
+ContentIndent = Token->getContentIndent(LineIndex);
+  LLVM_DEBUG(llvm::dbgs()
+ << "ContentIndent: " << ContentIndent << "\n");
+  ContentStartColumn = ContentIndent + Token->getContentStartColumn(
+   LineIndex, /*Break=*/true);
+
   unsigned NewRemainingTokenColumns = Token->getRemainingLength(
   LineIndex, TailOffset + Split.first + Split.second,
   ContentStartColumn);
+  if (NewRemainingTokenColumns == 0) {
+// No content to indent.
+ContentIndent = 0;
+ContentStartColumn =
+Token->getContentStartColumn(LineIndex, /*Break=*/true);
+NewRemainingTokenColumns = Token->getRemainingLength(
+LineIndex, TailOffset + Split.first + Split.second,
+ContentStartColumn);
+  }
 
   // When breaking before a tab character, it may be moved by a few columns,
   // but will still be expanded to the next tab stop, so we don't save any
@@ -1948,7 +1966,8 @@
   LLVM_DEBUG(llvm::dbgs() << "Breaking at: " << TailOffset + Split.first
   << ", " << Split.second << "\n");
   if (!DryRun)
-Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces);
+Token->insertBreak(LineIndex, TailOffset, Split, ContentIndent,
+   Whitespaces);
 
   Penalty += NewBreakPenalty;
   TailOffset += Split.first + Split.second;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -808,10 +808,9 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, triple slash directives (`/// <...`), @tag followed by { for a lot
-// of JSDoc tags, and @see, which is commonly followed by overlong URLs.
-GoogleStyle.CommentPragmas =
-"(taze:|^/[ \t]*<|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
+// taze:, triple slash directives (`/// <...`), @see, which is commonly
+// followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|^/[ \t]*<|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;
Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -21,6 +21,7 @@
 #include "Encoding.h"
 #include "TokenAnnotator.h"
 #include "WhitespaceManager.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Regex.h"
 #include 
 
@@ -135,6 +136,19 @@
   virtual unsigned getContentStartColumn(unsigned LineIndex,
  bool Break) const = 0;
 
+  /// Returns additional content indent required for the second line after the
+  /// content at line \p LineIndex is broken.
+  ///
+  /// For example, Javadoc @param annotations require and indent of 4 spaces and
+  /// in this example getContentIndex(1) returns 4.
+  /// /**
+  ///  * @param loong line
+  ///  * continuation
+  ///  */
+  virtual unsigned getContentIndent(unsigned LineIndex) const {
+return 0;
+  }
+
   /// Returns a range (offset, length) at which to break the line at

r338234 - [Analyzer] Iterator Checker Hotfix: Defer deletion of container data until its last iterator is cleaned up

2018-07-30 Thread Adam Balogh via cfe-commits
Author: baloghadamsoftware
Date: Mon Jul 30 01:52:21 2018
New Revision: 338234

URL: http://llvm.org/viewvc/llvm-project?rev=338234&view=rev
Log:
[Analyzer] Iterator Checker Hotfix: Defer deletion of container data until its 
last iterator is cleaned up

The analyzer may consider a container region as dead while it still has live
iterators. We must defer deletion of the data belonging to such containers
until all its iterators are dead as well to be able to compare the iterator
to the begin and the end of the container which is stored in the container
data.

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=338234&r1=338233&r2=338234&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Mon Jul 30 
01:52:21 2018
@@ -291,6 +291,7 @@ const ContainerData *getContainerData(Pr
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -536,7 +537,11 @@ void IteratorChecker::checkDeadSymbols(S
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  // We must keep the container data while it has live iterators to be able
+  // to compare them to the begin and the end of the container.
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1188,6 +1193,22 @@ ProgramStateRef relateIteratorPositions(
   return NewState;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
   auto &BVF = State->getBasicVals();
   return compare(State, Val,


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


[PATCH] D48427: [Analyzer] Iterator Checker Hotfix: Defer deletion of container data until its last iterator is cleaned up

2018-07-30 Thread Balogh , Ádám via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338234: [Analyzer] Iterator Checker Hotfix: Defer deletion 
of container data until its… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48427?vs=154066&id=157929#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48427

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -536,7 +537,11 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  // We must keep the container data while it has live iterators to be able
+  // to compare them to the begin and the end of the container.
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1188,6 +1193,22 @@
   return NewState;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
   auto &BVF = State->getBasicVals();
   return compare(State, Val,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -536,7 +537,11 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  // We must keep the container data while it has live iterators to be able
+  // to compare them to the begin and the end of the container.
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1188,6 +1193,22 @@
   return NewState;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
   auto &BVF = State->getBasicVals();
   return compare(State, Val,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49580: [clang-format] Adding style option for absolute formatting

2018-07-30 Thread Jacek Olesiak via Phabricator via cfe-commits
jolesiak added a comment.

First of all, thanks, Arnaud, for looking into this.

Let's address the mentioned bug first.
Let's assume that we use `IndentReference = Relative`. I think that this 
particular formatting bug is local (i.e. can be solved while processing a 
`@protocol` block). Look at this example:

  if (true) {
int a = 42;
  int b = 42;
int c = 42;
  }
  int d = 42;

When we run `clang-format -lines=4:4` we get:

  if (true) {
int a = 42;
  int b = 42;
  int c = 42;
  }
  int d = 42;

Please note that neither `}` nor `int d = 42` is indented. 
But this is a different behavior from what we see after running `clang-format 
-lines=3:3` on:

  @protocol A
   @optional
  // comment
  - (void)f;
  @end
  MACRO

the output is:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO

Please note that `@end` is indented (has different indentation than 
`@protocol`; `MACRO` is indented as well).
To clarify why this is undesired, `@end` ends `@protocol`, not `@optional`. By 
the way, `clang-format` doesn't think that `@end` should match `@optional`, 
check the output of an extended example:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO
   @end

Second `@end` still doesn't match the `@protocol`.

I think it's perfectly fine to allow users to have custom indentations if they 
so choose. In this particular example, though, `clang-format` should stop 
indenting when it reaches `@end`, giving the following output:

  @protocol A
   @optional
   // comment
   - (void)f;
  @end
  MACRO

Moving to the general case, I must say that introducing `IndentReference = 
Relative/Absolute` is a very interesting approach. I can imagine situations 
when somebody actually want to use `IndentReference = Absolute`. However, 
adding an additional option comes at quite big cost and I think that in this 
case it outweighs the benefits.
I think that a very good outcome could be to comment what is happening next to 
`IndentTracker.adjustToUnmodifiedLine(TheLine);` line. E.g. "We adjust an 
indentation to match the existing code to give users flexibility (instead of 
ignoring it). It's their responsibility to provide a correct formatting of 
lines they didn't initially change if these lines break formatting.".


https://reviews.llvm.org/D49580



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


[PATCH] D49580: [clang-format] Adding style option for absolute formatting

2018-07-30 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

Ok, so IIUC, understanding that @end effective ends a section much like "}" 
would address the currently observed problems?


https://reviews.llvm.org/D49580



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


r338238 - Fix -Wdocumentation warning. NFCI.

2018-07-30 Thread Simon Pilgrim via cfe-commits
Author: rksimon
Date: Mon Jul 30 03:07:47 2018
New Revision: 338238

URL: http://llvm.org/viewvc/llvm-project?rev=338238&view=rev
Log:
Fix -Wdocumentation warning. NFCI.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=338238&r1=338237&r2=338238&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Jul 30 03:07:47 2018
@@ -1654,7 +1654,6 @@ extern const internal::VariadicDynCastAl
 /// - (void) init {
 ///   a = @"hello";
 /// }
-//}
 /// \endcode
 extern const internal::VariadicDynCastAllOfMatcher
 objcIvarRefExpr;


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


[PATCH] D49580: [clang-format] Adding style option for absolute formatting

2018-07-30 Thread Jacek Olesiak via Phabricator via cfe-commits
jolesiak added a comment.

In https://reviews.llvm.org/D49580#1179924, @djasper wrote:

> Ok, so IIUC, understanding that @end effective ends a section much like "}" 
> would address the currently observed problems?


I think so.


https://reviews.llvm.org/D49580



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


[PATCH] D49289: [mips64][clang] Provide the signext attribute for i32 return values

2018-07-30 Thread Stefan Maksimovic via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338239: [mips64][clang] Provide the signext attribute for 
i32 return values (authored by smaksimovic, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D49289

Files:
  lib/CodeGen/TargetInfo.cpp


Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6985,8 +6985,14 @@
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
-   : ABIArgInfo::getDirect());
+  if (RetTy->isPromotableIntegerType())
+return ABIArgInfo::getExtend(RetTy);
+
+  if ((RetTy->isUnsignedIntegerOrEnumerationType() ||
+  RetTy->isSignedIntegerOrEnumerationType()) && Size == 32 && !IsO32)
+return ABIArgInfo::getSignExtend(RetTy);
+
+  return ABIArgInfo::getDirect();
 }
 
 void MipsABIInfo::computeInfo(CGFunctionInfo &FI) const {


Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6985,8 +6985,14 @@
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
-   : ABIArgInfo::getDirect());
+  if (RetTy->isPromotableIntegerType())
+return ABIArgInfo::getExtend(RetTy);
+
+  if ((RetTy->isUnsignedIntegerOrEnumerationType() ||
+  RetTy->isSignedIntegerOrEnumerationType()) && Size == 32 && !IsO32)
+return ABIArgInfo::getSignExtend(RetTy);
+
+  return ABIArgInfo::getDirect();
 }
 
 void MipsABIInfo::computeInfo(CGFunctionInfo &FI) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-07-30 Thread David CARLIER via Phabricator via cfe-commits
devnexen added a comment.

ping :)


https://reviews.llvm.org/D49722



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


Re: r337456 - [CodeGen] Disable aggressive structor optimizations at -O0, take 3

2018-07-30 Thread Pavel Labath via cfe-commits
Thank you for the explanation Chandler. This patch is not on a
critical path for me, so I don't mind it taking a while (though it
would be nice to fix as this is a also long standing cause of
expression evaluation failures in LLDB).

I believe that best solution here would be extend the set of
conditions under which we can emit the C1/D1 structor flavour as an
alias. There is no fundamental reason why these symbols cannot be
aliases even for linkonce linkage types. In fact, gcc will already
emit aliases in the situations we resort to separate functions.

The reason I could not do it in this patch (*) is that the emission of
the specific flavours for linkonce structors happens in a lazy matter
(i.e. only if they are referenced), but in order to correctly decide
what kind of emission strategy to choose, one needs to know the full
set of structors that will be emitted (**). This information is not
accessible (or at least I couldn't find a way to access it, maybe
someone with more knowledge of the codebase will) from the place which
does the decision.

If we are able to do that, then the object file size should be almost
unaffected (the only difference would be an extra symtab entry), and
it would also solve another issue, which this patch has inadvertently
caused: PR38338 - debugger stops twice for some constructor
breakpoints because of how we generate the line tables. In fact on
Friday, I was considering reverting this patch myself due to this
issue, but I eventually choose to leave it in as it did not seem too
hard to fix it going forward.

Let me know what you think about this idea. I am happy to do some of
the implementation work here, but I think I will need some guidance as
to how to collect and plumb the required information.

regards,
pavel


(*) It also didn't seem like it was necessary at the time, as this was
already strictly better than what we are doing on other platforms,
which is to emit separate functions unconditionally.

(**) If we don't emit a C1 alias, the C2 constructor needs to go to
the C2 comdat. If we emit the alias, we need to use the C5 comdat. The
situation is even more complicated with virtual destructors, as there
D0, D1 and D2 need to go to the same D5 comdat (and D1 can be an
alias). If D0 is missing/not emitted, then D2 needs to go to the D2
comdat, and D1 cannot be an alias.






On Sun, 29 Jul 2018 at 03:15, Chandler Carruth  wrote:
>
> On Sat, Jul 28, 2018 at 2:26 AM Chandler Carruth  wrote:
>>
>> On Thu, Jul 19, 2018 at 7:10 AM Pavel Labath via cfe-commits 
>>  wrote:
>>>
>>> Author: labath
>>> Date: Thu Jul 19 07:05:22 2018
>>> New Revision: 337456
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=337456&view=rev
>>> Log:
>>> [CodeGen] Disable aggressive structor optimizations at -O0, take 3
>>
>>
>> I'm really sorry to do this, but I need to revert this patch Let me try 
>> to explain
>>
>> We're seeing at least two issues with it that are causing *serious* issues 
>> for our users, and due to numerous other things, we really need to get 
>> top-of-tree into usable shape sooner rather than later. As this is improving 
>> a long-standing deficiency in Clang, I think it is reasonable to revert 
>> temporarily while we sort it out. I'm CC-ing Richard Smith and Eric 
>> Christopher directly as I'm going to ask them to make sure we get 
>> satisfactory answers to why this patch causes us so many problems and how we 
>> can make progress here.
>>
>> I don't have a test case at the moment, and I want to *very clearly* call 
>> out that it is on us to find a test case or otherwise clearly explain what 
>> problem this patch causes and how it can be addressed, or else this patch 
>> should be re-landed.
>>
>> To at least give some idea of what is going wrong here..
>>
>> First, this patch does increase object code size. This isn't really 
>> unexpected based on the nature of the patch, and it does so ostensibly in 
>> order to gain material fidelity improvements to debug information. Despite 
>> the fact that the increased object size causes us problems (it made a few 
>> hundered of our binaries' inputs too large to fit into the quota for the 
>> input files to our internal distributed build system) we tried to soldier 
>> onward...
>>
>> But it also causes the Gold linker at least to use considerably more memory 
>> than it used to. This has resulted in over 400 failures to link executables 
>> due to running out of available memory on the system.
>>
>> There are a number of possible causes for both the input size issues and the 
>> linker memory issue:
>> - An unexpected side-effect of this change causes lots of redundant sections 
>> to be output with -ffunction-sections, all of which merge away into nothing, 
>> but this takes huge amounts of object file and linker resources.
>> - The object file size increease is expected and unavoidable, but there is 
>> some bug in the linker being triggered?
>> - Something else
>
>
> I wanted to at least do some initial 

r338239 - [mips64][clang] Provide the signext attribute for i32 return values

2018-07-30 Thread Stefan Maksimovic via cfe-commits
Author: smaksimovic
Date: Mon Jul 30 03:44:46 2018
New Revision: 338239

URL: http://llvm.org/viewvc/llvm-project?rev=338239&view=rev
Log:
[mips64][clang] Provide the signext attribute for i32 return values

Additional info: see r338019.

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

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

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=338239&r1=338238&r2=338239&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Mon Jul 30 03:44:46 2018
@@ -6985,8 +6985,14 @@ ABIArgInfo MipsABIInfo::classifyReturnTy
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
-   : ABIArgInfo::getDirect());
+  if (RetTy->isPromotableIntegerType())
+return ABIArgInfo::getExtend(RetTy);
+
+  if ((RetTy->isUnsignedIntegerOrEnumerationType() ||
+  RetTy->isSignedIntegerOrEnumerationType()) && Size == 32 && !IsO32)
+return ABIArgInfo::getSignExtend(RetTy);
+
+  return ABIArgInfo::getDirect();
 }
 
 void MipsABIInfo::computeInfo(CGFunctionInfo &FI) const {


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


[PATCH] D47154: Try to make builtin address space declarations not useless

2018-07-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.

I missed the addr space casts  you added to CodeGenFunction::EmitBuiltinExpr. 
With those casts it should work.

For other downstream address space agnostic languages, e.g. (HCC), I guess they 
need to add similar hooks to use this feature.


https://reviews.llvm.org/D47154



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


[clang-tools-extra] r338241 - [clangd] Fix a comment. NFC

2018-07-30 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Mon Jul 30 04:46:25 2018
New Revision: 338241

URL: http://llvm.org/viewvc/llvm-project?rev=338241&view=rev
Log:
[clangd] Fix a comment. NFC

Modified:
clang-tools-extra/trunk/clangd/TUScheduler.cpp

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=338241&r1=338240&r2=338241&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Mon Jul 30 04:46:25 2018
@@ -367,11 +367,11 @@ void ASTWorker::update(
 PreambleWasBuilt.notify();
 
 if (CanReuseAST) {
-  // Take a shortcut and don't build the AST, neither the inputs nor the
+  // Take a shortcut and don't build the AST if neither the inputs nor the
   // preamble have changed.
   // Note that we do not report the diagnostics, since they should not have
   // changed either. All the clients should handle the lack of OnUpdated()
-  // call anyway, to handle empty result from buildAST.
+  // call anyway to handle empty result from buildAST.
   // FIXME(ibiryukov): the AST could actually change if non-preamble
   // includes changed, but we choose to ignore it.
   // FIXME(ibiryukov): should we refresh the cache in IdleASTs for the


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


r338245 - [clang-format] Silence -Wdocumentation warnings

2018-07-30 Thread Krasimir Georgiev via cfe-commits
Author: krasimir
Date: Mon Jul 30 05:22:41 2018
New Revision: 338245

URL: http://llvm.org/viewvc/llvm-project?rev=338245&view=rev
Log:
[clang-format] Silence -Wdocumentation warnings

introduced in r338232

Modified:
cfe/trunk/lib/Format/BreakableToken.h

Modified: cfe/trunk/lib/Format/BreakableToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=338245&r1=338244&r2=338245&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.h (original)
+++ cfe/trunk/lib/Format/BreakableToken.h Mon Jul 30 05:22:41 2018
@@ -139,12 +139,14 @@ public:
   /// Returns additional content indent required for the second line after the
   /// content at line \p LineIndex is broken.
   ///
-  /// For example, Javadoc @param annotations require and indent of 4 spaces 
and
-  /// in this example getContentIndex(1) returns 4.
-  /// /**
-  ///  * @param loong line
-  ///  * continuation
-  ///  */
+  // (Next lines do not start with `///` since otherwise -Wdocumentation picks
+  // up the example annotations and generates warnings for them)
+  // For example, Javadoc @param annotations require and indent of 4 spaces and
+  // in this example getContentIndex(1) returns 4.
+  // /**
+  //  * @param loong line
+  //  * continuation
+  //  */
   virtual unsigned getContentIndent(unsigned LineIndex) const {
 return 0;
   }


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


r338246 - [mips64][clang] Adjust tests to account for changes in r338239

2018-07-30 Thread Stefan Maksimovic via cfe-commits
Author: smaksimovic
Date: Mon Jul 30 05:27:40 2018
New Revision: 338246

URL: http://llvm.org/viewvc/llvm-project?rev=338246&view=rev
Log:
[mips64][clang] Adjust tests to account for changes in r338239


Modified:
cfe/trunk/test/CodeGen/mips-unsigned-ext-var.c
cfe/trunk/test/CodeGen/mips-varargs.c
cfe/trunk/test/CodeGen/mips-vector-arg.c

Modified: cfe/trunk/test/CodeGen/mips-unsigned-ext-var.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mips-unsigned-ext-var.c?rev=338246&r1=338245&r2=338246&view=diff
==
--- cfe/trunk/test/CodeGen/mips-unsigned-ext-var.c (original)
+++ cfe/trunk/test/CodeGen/mips-unsigned-ext-var.c Mon Jul 30 05:27:40 2018
@@ -17,6 +17,6 @@ void foo1() {
   foo(1,f);
 }
 
-//N64: call i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)
-//N32: call i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)
-//O32: call i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)
\ No newline at end of file
+//N64: call signext i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)
+//N32: call signext i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)
+//O32: call i32 (i32, ...) @foo(i32 signext undef, i32 signext -32)

Modified: cfe/trunk/test/CodeGen/mips-varargs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mips-varargs.c?rev=338246&r1=338245&r2=338246&view=diff
==
--- cfe/trunk/test/CodeGen/mips-varargs.c (original)
+++ cfe/trunk/test/CodeGen/mips-varargs.c Mon Jul 30 05:27:40 2018
@@ -19,7 +19,9 @@ int test_i32(char *fmt, ...) {
   return v;
 }
 
-// ALL-LABEL: define i32 @test_i32(i8*{{.*}} %fmt, ...)
+// O32-LABEL: define i32 @test_i32(i8*{{.*}} %fmt, ...)
+// N32-LABEL: define signext i32 @test_i32(i8*{{.*}} %fmt, ...)
+// N64-LABEL: define signext i32 @test_i32(i8*{{.*}} %fmt, ...)
 //
 // O32:   %va = alloca i8*, align [[$PTRALIGN:4]]
 // N32:   %va = alloca i8*, align [[$PTRALIGN:4]]
@@ -133,7 +135,9 @@ int test_v4i32(char *fmt, ...) {
   return v[0];
 }
 
-// ALL-LABEL: define i32 @test_v4i32(i8*{{.*}} %fmt, ...)
+// O32-LABEL: define i32 @test_v4i32(i8*{{.*}} %fmt, ...)
+// N32-LABEL: define signext i32 @test_v4i32(i8*{{.*}} %fmt, ...)
+// N64-LABEL: define signext i32 @test_v4i32(i8*{{.*}} %fmt, ...)
 //
 // ALL:   %va = alloca i8*, align [[$PTRALIGN]]
 // ALL:   [[V:%.+]] = alloca <4 x i32>, align 16

Modified: cfe/trunk/test/CodeGen/mips-vector-arg.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mips-vector-arg.c?rev=338246&r1=338245&r2=338246&view=diff
==
--- cfe/trunk/test/CodeGen/mips-vector-arg.c (original)
+++ cfe/trunk/test/CodeGen/mips-vector-arg.c Mon Jul 30 05:27:40 2018
@@ -11,7 +11,7 @@ typedef int v4i32 __attribute__ ((__vect
 // O32: define void @test_v4sf(i32 inreg %a1.coerce0, i32 inreg %a1.coerce1, 
i32 inreg %a1.coerce2, i32 inreg %a1.coerce3, i32 signext %a2, i32, i32 inreg 
%a3.coerce0, i32 inreg %a3.coerce1, i32 inreg %a3.coerce2, i32 inreg 
%a3.coerce3) local_unnamed_addr [[NUW:#[0-9]+]]
 // O32: declare i32 @test_v4sf_2(i32 inreg, i32 inreg, i32 inreg, i32 inreg, 
i32 signext, i32, i32 inreg, i32 inreg, i32 inreg, i32 inreg)
 // N64: define void @test_v4sf(i64 inreg %a1.coerce0, i64 inreg %a1.coerce1, 
i32 signext %a2, i64, i64 inreg %a3.coerce0, i64 inreg %a3.coerce1) 
local_unnamed_addr [[NUW:#[0-9]+]]
-// N64: declare i32 @test_v4sf_2(i64 inreg, i64 inreg, i32 signext, i64, i64 
inreg, i64 inreg)
+// N64: declare signext i32 @test_v4sf_2(i64 inreg, i64 inreg, i32 signext, 
i64, i64 inreg, i64 inreg)
 extern test_v4sf_2(v4sf, int, v4sf);
 void test_v4sf(v4sf a1, int a2, v4sf a3) {
   test_v4sf_2(a3, a2, a1);
@@ -20,7 +20,7 @@ void test_v4sf(v4sf a1, int a2, v4sf a3)
 // O32: define void @test_v4i32(i32 inreg %a1.coerce0, i32 inreg %a1.coerce1, 
i32 inreg %a1.coerce2, i32 inreg %a1.coerce3, i32 signext %a2, i32, i32 inreg 
%a3.coerce0, i32 inreg %a3.coerce1, i32 inreg %a3.coerce2, i32 inreg 
%a3.coerce3) local_unnamed_addr [[NUW]]
 // O32: declare i32 @test_v4i32_2(i32 inreg, i32 inreg, i32 inreg, i32 inreg, 
i32 signext, i32, i32 inreg, i32 inreg, i32 inreg, i32 inreg)
 // N64: define void @test_v4i32(i64 inreg %a1.coerce0, i64 inreg %a1.coerce1, 
i32 signext %a2, i64, i64 inreg %a3.coerce0, i64 inreg %a3.coerce1) 
local_unnamed_addr [[NUW]]
-// N64: declare i32 @test_v4i32_2(i64 inreg, i64 inreg, i32 signext, i64, i64 
inreg, i64 inreg)
+// N64: declare signext i32 @test_v4i32_2(i64 inreg, i64 inreg, i32 signext, 
i64, i64 inreg, i64 inreg)
 extern test_v4i32_2(v4i32, int, v4i32);
 void test_v4i32(v4i32 a1, int a2, v4i32 a3) {
   test_v4i32_2(a3, a2, a1);


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


[PATCH] D49986: [ADT] ImmutableList::add parameters are switched

2018-07-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, dcoughlin, chandlerc.
Herald added a subscriber: cfe-commits.

Clang side changes, see https://reviews.llvm.org/D49985.


Repository:
  rC Clang

https://reviews.llvm.org/D49986

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
@@ -153,7 +153,7 @@
 using context_type = typename data_type::Factory &;
 
 static data_type Add(data_type L, key_type K, context_type F) {
-  return F.add(L, K);
+  return F.add(K, L);
 }
 
 static bool Contains(data_type L, key_type K) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -233,7 +233,7 @@
   }
 
   llvm::ImmutableList prependSVal(SVal X, llvm::ImmutableList L) {
-return SValListFactory.add(L, X);
+return SValListFactory.add(X, L);
   }
 
   llvm::ImmutableList getEmptyCXXBaseList() {
@@ -243,7 +243,7 @@
   llvm::ImmutableList prependCXXBase(
   const CXXBaseSpecifier *CBS,
   llvm::ImmutableList L) {
-return CXXBaseListFactory.add(L, CBS);
+return CXXBaseListFactory.add(CBS, L);
   }
 
   const PointerToMemberData *accumCXXBase(


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h
@@ -153,7 +153,7 @@
 using context_type = typename data_type::Factory &;
 
 static data_type Add(data_type L, key_type K, context_type F) {
-  return F.add(L, K);
+  return F.add(K, L);
 }
 
 static bool Contains(data_type L, key_type K) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -233,7 +233,7 @@
   }
 
   llvm::ImmutableList prependSVal(SVal X, llvm::ImmutableList L) {
-return SValListFactory.add(L, X);
+return SValListFactory.add(X, L);
   }
 
   llvm::ImmutableList getEmptyCXXBaseList() {
@@ -243,7 +243,7 @@
   llvm::ImmutableList prependCXXBase(
   const CXXBaseSpecifier *CBS,
   llvm::ImmutableList L) {
-return CXXBaseListFactory.add(L, CBS);
+return CXXBaseListFactory.add(CBS, L);
   }
 
   const PointerToMemberData *accumCXXBase(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

I am not in favor of this patch.

I'm in favor of fixing the problem that Arthur has described, but not like 
this, for the following reasons:

- Conceptually, these are (similar to) "Allocator-based versions of the 
algorithms proposed in P0040 ", and should (probably? 
possibly?) look more like them.

- Mainly, though, I think that the goal of this patch (which is see as 'getting 
to memcpy') is not the direction that libc++ should take.  Instead, we should 
be writing simple loops that the compiler can optimize into a call to memcpy if 
it chooses. Having calls to `memcpy` in the code paths makes it impossible to 
"constexp-ify" this code. (See https://libcxx.llvm.org/cxx2a_status.html 
(comments on `std::copy` and https://bugs.llvm.org/show_bug.cgi?id=25165).


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D49244: Always search sysroot for GCC installs

2018-07-30 Thread David Greene via Phabricator via cfe-commits
greened added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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


[PATCH] D49898: Make test/Driver/baremetal.cpp work with linkers other than lld

2018-07-30 Thread David Greene via Phabricator via cfe-commits
greened added a comment.

I think I need an accept before this can proceed.


Repository:
  rC Clang

https://reviews.llvm.org/D49898



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


[PATCH] D49075: [NEON] Define fp16 vld and vst intrinsics conditionally

2018-07-30 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev updated this revision to Diff 157960.
kosarev added a comment.

Test that the affected intrinsics are not defined in no-fp16 mode.


https://reviews.llvm.org/D49075

Files:
  include/clang/Basic/arm_neon.td
  test/CodeGen/arm-neon-vld.c
  test/CodeGen/arm-neon-vst.c
  test/Sema/arm-no-fp16.c

Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -83,3 +83,213 @@
 float16x8_t test_vminnmq_f16(float16x8_t a, float16x8_t b) {
   return vminnmq_f16(a, b); // expected-warning{{implicit declaration of function 'vminnmq_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
 }
+
+float16x4_t test_vld1_f16(const float16_t *a) {
+  return vld1_f16(a); // expected-warning{{implicit declaration of function 'vld1_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vld1q_f16(const float16_t *a) {
+  return vld1q_f16(a); // expected-warning{{implicit declaration of function 'vld1q_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vld1_dup_f16(const float16_t *a) {
+  return vld1_dup_f16(a); // expected-warning{{implicit declaration of function 'vld1_dup_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vld1q_dup_f16(const float16_t *a) {
+  return vld1q_dup_f16(a); // expected-warning{{implicit declaration of function 'vld1q_dup_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vld1_lane_f16(const float16_t *a, float16x4_t b) {
+  return vld1_lane_f16(a, b, 3); // expected-warning{{implicit declaration of function 'vld1_lane_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vld1q_lane_f16(const float16_t *a, float16x8_t b) {
+  return vld1q_lane_f16(a, b, 7); // expected-warning{{implicit declaration of function 'vld1q_lane_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
+
+float16x4x2_t test_vld1_f16_x2(const float16_t *a) {
+  return vld1_f16_x2(a); // expected-warning{{implicit declaration of function 'vld1_f16_x2'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4x2_t'}}
+}
+
+float16x8x2_t test_vld1q_f16_x2(const float16_t *a) {
+  return vld1q_f16_x2(a); // expected-warning{{implicit declaration of function 'vld1q_f16_x2'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8x2_t'}}
+}
+
+float16x4x3_t test_vld1_f16_x3(const float16_t *a) {
+  return vld1_f16_x3(a); // expected-warning{{implicit declaration of function 'vld1_f16_x3'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4x3_t'}}
+}
+
+float16x8x3_t test_vld1q_f16_x3(const float16_t *a) {
+  return vld1q_f16_x3(a); // expected-warning{{implicit declaration of function 'vld1q_f16_x3'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8x3_t'}}
+}
+
+float16x4x4_t test_vld1_f16_x4(const float16_t *a) {
+  return vld1_f16_x4(a); // expected-warning{{implicit declaration of function 'vld1_f16_x4'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4x4_t'}}
+}
+
+float16x8x4_t test_vld1q_f16_x4(const float16_t *a) {
+  return vld1q_f16_x4(a); // expected-warning{{implicit declaration of function 'vld1q_f16_x4'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8x4_t'}}
+}
+
+float16x4x2_t test_vld2_f16(const float16_t *a) {
+  return vld2_f16(a); // expected-warning{{implicit declaration of function 'vld2_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4x2_t'}}
+}
+
+float16x8x2_t test_vld2q_f16(const float16_t *a) {
+  return vld2q_f16(a); // expected-warning{{implicit declaration of function 'vld2q_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8x2_t'}}
+}
+
+float16x4x2_t test_vld2_lane_f16(const float16_t *a, float16x4x2_t b) {
+  return vld2_lane_f16(a, b, 3); // expected-warning{{implicit declaration of function 'vld2_lane_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4x2_t'}}
+}
+
+float16x8x2_t test_vld2q_lane_f16(const float16_t *a, float16x8x2_t b) {
+  return vld2q_lane_f16(a, b, 7); // expected-warning{{implicit declaration of function 'vld2q_lane_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8x2_t'}}
+}
+
+float16x4x2_t test_vld2_dup_f16(const float16_t *src) {
+  return vld2_dup_f16(src); // expected-warning{{implicit declaration of function 'vld2_dup_f16'}} expecte

r338252 - [OPENMP] Modify the info about OpenMP support in UsersManual, NFC.

2018-07-30 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Mon Jul 30 07:44:29 2018
New Revision: 338252

URL: http://llvm.org/viewvc/llvm-project?rev=338252&view=rev
Log:
[OPENMP] Modify the info about OpenMP support in UsersManual, NFC.

Modified:
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=338252&r1=338251&r2=338252&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Jul 30 07:44:29 2018
@@ -2155,13 +2155,8 @@ Objective-C++ Language Features
 OpenMP Features
 ===
 
-Clang supports all OpenMP 3.1 directives and clauses.  In addition, some
-features of OpenMP 4.0 are supported.  For example, ``#pragma omp simd``,
-``#pragma omp for simd``, ``#pragma omp parallel for simd`` directives, 
extended
-set of atomic constructs, ``proc_bind`` clause for all parallel-based
-directives, ``depend`` clause for ``#pragma omp task`` directive (except for
-array sections), ``#pragma omp cancel`` and ``#pragma omp cancellation point``
-directives, and ``#pragma omp taskgroup`` directive.
+Clang supports all OpenMP 4.5 directives and clauses. See :doc:`OpenMPSupport`
+for additional details.
 
 Use `-fopenmp` to enable OpenMP. Support for OpenMP can be disabled with
 `-fno-openmp`.


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


[PATCH] D49989: WIP: [CodeGen] Disable aggressive structor optimizations at -O0, take 4

2018-07-30 Thread Pavel Labath via Phabricator via cfe-commits
labath created this revision.
Herald added subscribers: cfe-commits, aheejin.

This version of the patch attempts to mitigate the code size regressions
caused by the previous versions of this patch 
(https://reviews.llvm.org/D46685). It does this by
using aliases and C5/https://reviews.llvm.org/D5 comdats more aggressively.

This is not completely trivial, as linkonce_odr structors (these are
othe ones that were previously causing the blowup) are emitte lazily,
only when they are referenced, but we are only able to use the comdat if
all of the elements that should go into it are emitted. This is
particularly tricky for virtual desctructors as here the comdat must
include all three versions (complete, base, deleting).

As we do not (?) want this to impact the decisions on whether to emit
certain variants or not (and the full set of emitted versions is not
easily accessible from the place where we make the comdat decision), we
have to opportunistically wait for all required versions to be emitted.
If they are, we coalesce them into a comdat.


Repository:
  rC Clang

https://reviews.llvm.org/D49989

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/ctor-dtor-alias.cpp
  test/CodeGenCXX/float16-declarations.cpp

Index: test/CodeGenCXX/float16-declarations.cpp
===
--- test/CodeGenCXX/float16-declarations.cpp
+++ test/CodeGenCXX/float16-declarations.cpp
@@ -103,7 +103,7 @@
 
   C1 c1(f1l);
 // CHECK-DAG:  [[F1L:%[a-z0-9]+]] = load half, half* %{{.*}}, align 2
-// CHECK-DAG:  call void @_ZN2C1C2EDF16_(%class.C1* %{{.*}}, half %{{.*}})
+// CHECK-DAG:  call void @_ZN2C1C1EDF16_(%class.C1* %{{.*}}, half %{{.*}})
 
   S1<_Float16> s1 = { 132.f16 };
 // CHECK-DAG: @_ZZ4mainE2s1 = private unnamed_addr constant %struct.S1 { half 0xH5820 }, align 2
Index: test/CodeGenCXX/ctor-dtor-alias.cpp
===
--- test/CodeGenCXX/ctor-dtor-alias.cpp
+++ test/CodeGenCXX/ctor-dtor-alias.cpp
@@ -1,5 +1,11 @@
-// RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases | FileCheck --check-prefix=NOOPT %s
-
+// RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases > %t
+// RUN: FileCheck --check-prefix=NOOPT1 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT2 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT3 --implicit-check-not=_ZN6test2a6foobarIvED0Ev --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT4 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT5 --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT6 --implicit-check-not=_ZN6test4a1BD0Ev --input-file=%t %s
+// RUN: FileCheck --check-prefix=NOOPT7 --input-file=%t %s
 // RUN: %clang_cc1 %s -triple i686-linux -emit-llvm -o - -mconstructor-aliases -O1 -disable-llvm-passes > %t
 // RUN: FileCheck --check-prefix=CHECK1 --input-file=%t %s
 // RUN: FileCheck --check-prefix=CHECK2 --input-file=%t %s
@@ -21,6 +27,13 @@
 // CHECK1: define weak_odr void @_ZN5test16foobarIvED0Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
 // CHECK1-NOT: comdat
 
+// This should happen regardless of the opt level.
+// NOOPT1: @_ZN5test16foobarIvEC1Ev = weak_odr unnamed_addr alias void {{.*}} @_ZN5test16foobarIvEC2Ev
+// NOOPT1: @_ZN5test16foobarIvED1Ev = weak_odr unnamed_addr alias void (%"struct.test1::foobar"*), void (%"struct.test1::foobar"*)* @_ZN5test16foobarIvED2Ev
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvEC2Ev({{.*}} comdat($_ZN5test16foobarIvEC5Ev)
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvED2Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
+// NOOPT1: define weak_odr void @_ZN5test16foobarIvED0Ev({{.*}} comdat($_ZN5test16foobarIvED5Ev)
+
 // COFF doesn't support comdats with arbitrary names (C5/D5).
 // COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvEC2Ev({{.*}} comdat align
 // COFF: define weak_odr {{.*}} void @_ZN5test16foobarIvEC1Ev({{.*}} comdat align
@@ -37,26 +50,52 @@
 }
 
 namespace test2 {
-// test that when the destrucor is linkonce_odr we just replace every use of
+// test that when the destructor is linkonce_odr we just replace every use of
 // C1 with C2.
 
 // CHECK1: define internal void @__cxx_global_var_init()
 // CHECK1: call void @_ZN5test26foobarIvEC2Ev
 // CHECK1: define linkonce_odr void @_ZN5test26foobarIvEC2Ev({{.*}} comdat align
+
+// At -O0, we still emit both symbols, but C1 is an alias to save space.
+// NOOPT2: @_ZN5test26foobarIvEC1Ev = linkonce_odr unnamed_addr alias void {{.*}} @_ZN5test26foobarIvEC2Ev
+// NOOPT2: define internal void @__cxx_global_var_init()
+// NOOPT2: call void @_ZN5test26foobarIvEC1Ev
+// NOOPT2: define linkonce_odr void @_ZN5test26foobarIvEC2Ev({{.*}} comdat($_ZN5test26foobarIvEC5Ev) align
 void g();
 template  struct foobar {
   foobar() { g(); }
 };
 foobar x;
 }
 
+namespace test2a {
+// Similar to test2, but check destructors. As D0 is not referenced and not
+// virtual, it shouldn't be emitted, nor placed into a co

[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker

2018-07-30 Thread Reka Kovacs via Phabricator via cfe-commits
rnkovacs updated this revision to Diff 157966.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.

De-duplicate & add comment.


https://reviews.llvm.org/D49361

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/inner-pointer.cpp
  test/Analysis/malloc-free-after-return.cpp


Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+  S() : Data(new int) {}
+  ~S() { delete Data; }
+  int *getData() { return Data; }
+
+private:
+  int *Data;
+};
+
+int *freeAfterReturnTemp() {
+  return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+  S X;
+  return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
   consume(c);// expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
+
+struct S {
+  std::string to_string() { return s; }
+private:
+  std::string s;
+};
+
+const char *escape_via_return_temp() {
+  S x;
+  return x.to_string().c_str(); // expected-note {{Dangling inner pointer 
obtained here}}
+  // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+  // expected-warning@-2 {{Use of memory after it is freed}}
+  // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+  std::string s;
+  return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to 
destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
  check::PointerEscape,
  check::ConstPointerEscape,
  check::PreStmt,
+ check::EndFunction,
  check::PreCall,
  check::PostStmt,
  check::PostStmt,
@@ -217,6 +218,7 @@
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
 bool Assumption) const;
   void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -2475,6 +2477,17 @@
 checkUseAfterFree(Sym, C, E);
 }
 
+// In the CFG, automatic destructors come after the return statement.
+// This callback checks for returning memory that is freed by automatic
+// destructors, as those cannot be reached from `checkPreStmt()`.
+void MallocChecker::checkEndFunction(const ReturnStmt *S,
+ CheckerContext &C) const {
+  if (!S)
+return;
+
+  checkPreStmt(S, C);
+}
+
 // TODO: Blocks should be either inlined or should call invalidate regions
 // upon invocation. After that's in place, special casing here will not be
 // needed.


Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+  S() : Data(new int) {}
+  ~S() { delete Data; }
+  int *getData() { return Data; }
+
+private:
+  int *Data;
+};
+
+int *freeAfterReturnTemp() {
+  return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+  S X;
+  return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
   consume(c);// expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
+
+struct S {
+  std::string to_string() { return s; }
+private:
+  std::string s;
+};
+
+const cha

Re: r337456 - [CodeGen] Disable aggressive structor optimizations at -O0, take 3

2018-07-30 Thread Pavel Labath via cfe-commits
I've managed to hack together an implementation of this proposal. I've
put it up at  so you have an idea of
where I'm going with this. It seems to work fine (I've managed to
bootstrap clang with it), though I'm not exactly proud of the
implementation.

Let me know what you think.

pl
On Mon, 30 Jul 2018 at 12:06, Pavel Labath  wrote:
>
> Thank you for the explanation Chandler. This patch is not on a
> critical path for me, so I don't mind it taking a while (though it
> would be nice to fix as this is a also long standing cause of
> expression evaluation failures in LLDB).
>
> I believe that best solution here would be extend the set of
> conditions under which we can emit the C1/D1 structor flavour as an
> alias. There is no fundamental reason why these symbols cannot be
> aliases even for linkonce linkage types. In fact, gcc will already
> emit aliases in the situations we resort to separate functions.
>
> The reason I could not do it in this patch (*) is that the emission of
> the specific flavours for linkonce structors happens in a lazy matter
> (i.e. only if they are referenced), but in order to correctly decide
> what kind of emission strategy to choose, one needs to know the full
> set of structors that will be emitted (**). This information is not
> accessible (or at least I couldn't find a way to access it, maybe
> someone with more knowledge of the codebase will) from the place which
> does the decision.
>
> If we are able to do that, then the object file size should be almost
> unaffected (the only difference would be an extra symtab entry), and
> it would also solve another issue, which this patch has inadvertently
> caused: PR38338 - debugger stops twice for some constructor
> breakpoints because of how we generate the line tables. In fact on
> Friday, I was considering reverting this patch myself due to this
> issue, but I eventually choose to leave it in as it did not seem too
> hard to fix it going forward.
>
> Let me know what you think about this idea. I am happy to do some of
> the implementation work here, but I think I will need some guidance as
> to how to collect and plumb the required information.
>
> regards,
> pavel
>
>
> (*) It also didn't seem like it was necessary at the time, as this was
> already strictly better than what we are doing on other platforms,
> which is to emit separate functions unconditionally.
>
> (**) If we don't emit a C1 alias, the C2 constructor needs to go to
> the C2 comdat. If we emit the alias, we need to use the C5 comdat. The
> situation is even more complicated with virtual destructors, as there
> D0, D1 and D2 need to go to the same D5 comdat (and D1 can be an
> alias). If D0 is missing/not emitted, then D2 needs to go to the D2
> comdat, and D1 cannot be an alias.
>
>
>
>
>
>
> On Sun, 29 Jul 2018 at 03:15, Chandler Carruth  wrote:
> >
> > On Sat, Jul 28, 2018 at 2:26 AM Chandler Carruth  
> > wrote:
> >>
> >> On Thu, Jul 19, 2018 at 7:10 AM Pavel Labath via cfe-commits 
> >>  wrote:
> >>>
> >>> Author: labath
> >>> Date: Thu Jul 19 07:05:22 2018
> >>> New Revision: 337456
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=337456&view=rev
> >>> Log:
> >>> [CodeGen] Disable aggressive structor optimizations at -O0, take 3
> >>
> >>
> >> I'm really sorry to do this, but I need to revert this patch Let me 
> >> try to explain
> >>
> >> We're seeing at least two issues with it that are causing *serious* issues 
> >> for our users, and due to numerous other things, we really need to get 
> >> top-of-tree into usable shape sooner rather than later. As this is 
> >> improving a long-standing deficiency in Clang, I think it is reasonable to 
> >> revert temporarily while we sort it out. I'm CC-ing Richard Smith and Eric 
> >> Christopher directly as I'm going to ask them to make sure we get 
> >> satisfactory answers to why this patch causes us so many problems and how 
> >> we can make progress here.
> >>
> >> I don't have a test case at the moment, and I want to *very clearly* call 
> >> out that it is on us to find a test case or otherwise clearly explain what 
> >> problem this patch causes and how it can be addressed, or else this patch 
> >> should be re-landed.
> >>
> >> To at least give some idea of what is going wrong here..
> >>
> >> First, this patch does increase object code size. This isn't really 
> >> unexpected based on the nature of the patch, and it does so ostensibly in 
> >> order to gain material fidelity improvements to debug information. Despite 
> >> the fact that the increased object size causes us problems (it made a few 
> >> hundered of our binaries' inputs too large to fit into the quota for the 
> >> input files to our internal distributed build system) we tried to soldier 
> >> onward...
> >>
> >> But it also causes the Gold linker at least to use considerably more 
> >> memory than it used to. This has resulted in over 400 failures to link 
> >> executables due to running ou

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

2018-07-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

This LGTM, but suggests another cleanup.

We have a bunch of `_LIBCPP_BUILDING_XXX` macros defined in libcxx/src - do we 
need them any more now that we have `_LIBCPP_BUILDING_LIBRARY`?

  src/bind.cpp:10: #define _LIBCPP_BUILDING_BIND
  src/memory.cpp:10: #define _LIBCPP_BUILDING_MEMORY
  src/mutex.cpp:10: #define _LIBCPP_BUILDING_MUTEX
  src/new.cpp:10: #define _LIBCPP_BUILDING_NEW
  src/shared_mutex.cpp:13: #define _LIBCPP_BUILDING_SHARED_MUTEX
  src/system_error.cpp:12: #define _LIBCPP_BUILDING_SYSTEM_ERROR
  src/utility.cpp:10: #define _LIBCPP_BUILDING_UTILITY

I can take care of this post-branch for release.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49914



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


r338255 - [CodeComplete] Fix the crash in code completion on access checking

2018-07-30 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Mon Jul 30 08:19:05 2018
New Revision: 338255

URL: http://llvm.org/viewvc/llvm-project?rev=338255&view=rev
Log:
[CodeComplete] Fix the crash in code completion on access checking

Started crashing in r337453. See the added test case for the crash repro.

The fix reverts part of r337453 that causes the crash and does
not actually break anything when reverted.

Added:
cfe/trunk/test/Index/complete-access-checks-crash.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=338255&r1=338254&r2=338255&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Jul 30 08:19:05 2018
@@ -1303,34 +1303,8 @@ namespace {
 void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
bool InBaseClass) override {
   bool Accessible = true;
-  if (Ctx) {
-DeclContext *AccessingCtx = Ctx;
-// If ND comes from a base class, set the naming class back to the
-// derived class if the search starts from the derived class (i.e.
-// InBaseClass is true).
-//
-// Example:
-//   class B { protected: int X; }
-//   class D : public B { void f(); }
-//   void D::f() { this->^; }
-// The completion after "this->" will have `InBaseClass` set to true 
and
-// `Ctx` set to "B", when looking up in `B`. We need to set the actual
-// accessing context (i.e. naming class) to "D" so that access can be
-// calculated correctly.
-if (InBaseClass && isa(Ctx)) {
-  CXXRecordDecl *RC = nullptr;
-  // Get the enclosing record.
-  for (DeclContext *DC = CurContext; !DC->isFileContext();
-   DC = DC->getParent()) {
-if ((RC = dyn_cast(DC)))
-  break;
-  }
-  if (RC)
-AccessingCtx = RC;
-}
-Accessible = Results.getSema().IsSimplyAccessible(ND, AccessingCtx);
-  }
-
+  if (Ctx)
+Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
   ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
false, Accessible, FixIts);
   Results.AddResult(Result, CurContext, Hiding, InBaseClass);

Added: cfe/trunk/test/Index/complete-access-checks-crash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-access-checks-crash.cpp?rev=338255&view=auto
==
--- cfe/trunk/test/Index/complete-access-checks-crash.cpp (added)
+++ cfe/trunk/test/Index/complete-access-checks-crash.cpp Mon Jul 30 08:19:05 
2018
@@ -0,0 +1,13 @@
+struct Base {
+protected:
+  bool bar();
+};
+struct Derived : Base {
+};
+
+struct X {
+  int foo() {
+Derived(). // RUN: c-index-test -code-completion-at=%s:10:15 %s | 
FileCheck %s
+// CHECK: bar{{.*}}(inaccessible)
+  }
+};


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


Re: r337453 - [CodeComplete] Fix accessibilty of protected members from base class.

2018-07-30 Thread Ilya Biryukov via cfe-commits
Prtially reverted the commit in r338255 to fix a very frequent crash.
The code seemed obviously wrong there (calling accessibility checking,
passing a possibly unrelated class) and the tests didn't break after
reverting it.

Let me know if I missed anything.

On Thu, Jul 19, 2018 at 3:37 PM Eric Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ioeric
> Date: Thu Jul 19 06:32:00 2018
> New Revision: 337453
>
> URL: http://llvm.org/viewvc/llvm-project?rev=337453&view=rev
> Log:
> [CodeComplete] Fix accessibilty of protected members from base class.
>
> Summary:
> Currently, protected members from base classes are marked as
> inaccessible when completing in derived class. This patch fixes the
> problem by
> setting the naming class correctly when looking up results in base class
> according to [11.2.p5].
>
> Reviewers: aaron.ballman, sammccall, rsmith
>
> Reviewed By: aaron.ballman
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D49421
>
> Modified:
> cfe/trunk/lib/Sema/SemaAccess.cpp
> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> cfe/trunk/test/Index/complete-access-checks.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaAccess.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=337453&r1=337452&r2=337453&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaAccess.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaAccess.cpp Thu Jul 19 06:32:00 2018
> @@ -11,6 +11,7 @@
>  //
>
>  
> //===--===//
>
> +#include "clang/Basic/Specifiers.h"
>  #include "clang/Sema/SemaInternal.h"
>  #include "clang/AST/ASTContext.h"
>  #include "clang/AST/CXXInheritance.h"
> @@ -1856,29 +1857,31 @@ void Sema::CheckLookupAccess(const Looku
>}
>  }
>
> -/// Checks access to Decl from the given class. The check will take access
> +/// Checks access to Target from the given class. The check will take
> access
>  /// specifiers into account, but no member access expressions and such.
>  ///
> -/// \param Decl the declaration to check if it can be accessed
> +/// \param Target the declaration to check if it can be accessed
>  /// \param Ctx the class/context from which to start the search
> -/// \return true if the Decl is accessible from the Class, false
> otherwise.
> -bool Sema::IsSimplyAccessible(NamedDecl *Decl, DeclContext *Ctx) {
> +/// \return true if the Target is accessible from the Class, false
> otherwise.
> +bool Sema::IsSimplyAccessible(NamedDecl *Target, DeclContext *Ctx) {
>if (CXXRecordDecl *Class = dyn_cast(Ctx)) {
> -if (!Decl->isCXXClassMember())
> +if (!Target->isCXXClassMember())
>return true;
>
> +if (Target->getAccess() == AS_public)
> +  return true;
>  QualType qType = Class->getTypeForDecl()->getCanonicalTypeInternal();
> +// The unprivileged access is AS_none as we don't know how the member
> was
> +// accessed, which is described by the access in DeclAccessPair.
> +// `IsAccessible` will examine the actual access of Target (i.e.
> +// Decl->getAccess()) when calculating the access.
>  AccessTarget Entity(Context, AccessedEntity::Member, Class,
> -DeclAccessPair::make(Decl, Decl->getAccess()),
> -qType);
> -if (Entity.getAccess() == AS_public)
> -  return true;
> -
> +DeclAccessPair::make(Target, AS_none), qType);
>  EffectiveContext EC(CurContext);
>  return ::IsAccessible(*this, EC, Entity) != ::AR_inaccessible;
>}
> -
> -  if (ObjCIvarDecl *Ivar = dyn_cast(Decl)) {
> +
> +  if (ObjCIvarDecl *Ivar = dyn_cast(Target)) {
>  // @public and @package ivars are always accessible.
>  if (Ivar->getCanonicalAccessControl() == ObjCIvarDecl::Public ||
>  Ivar->getCanonicalAccessControl() == ObjCIvarDecl::Package)
>
> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=337453&r1=337452&r2=337453&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Jul 19 06:32:00 2018
> @@ -1303,8 +1303,33 @@ namespace {
>  void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
> bool InBaseClass) override {
>bool Accessible = true;
> -  if (Ctx)
> -Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
> +  if (Ctx) {
> +DeclContext *AccessingCtx = Ctx;
> +// If ND comes from a base class, set the naming class back to the
> +// derived class if the search starts from the derived class (i.e.
> +// InBaseClass is true).
> +//
> +// Example:
> +//   class B { protected: int X; }
> +//   class D : public B 

[clang-tools-extra] r338256 - [clangd] Do not remove AST from cache if nothing changed

2018-07-30 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Mon Jul 30 08:30:45 2018
New Revision: 338256

URL: http://llvm.org/viewvc/llvm-project?rev=338256&view=rev
Log:
[clangd] Do not remove AST from cache if nothing changed

We were previously clearing the AST cache if the inputs and the
preamble were the same, which is not desired.

Modified:
clang-tools-extra/trunk/clangd/TUScheduler.cpp

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=338256&r1=338255&r2=338256&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Mon Jul 30 08:30:45 2018
@@ -331,8 +331,6 @@ void ASTWorker::update(
 
 tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand);
 FileInputs = Inputs;
-// Remove the old AST if it's still in cache.
-IdleASTs.take(this);
 
 log("Updating file {0} with command [{1}] {2}", FileName,
 Inputs.CompileCommand.Directory,
@@ -342,6 +340,8 @@ void ASTWorker::update(
 buildCompilerInvocation(Inputs);
 if (!Invocation) {
   elog("Could not build CompilerInvocation for file {0}", FileName);
+  // Remove the old AST if it's still in cache.
+  IdleASTs.take(this);
   // Make sure anyone waiting for the preamble gets notified it could not
   // be built.
   PreambleWasBuilt.notify();
@@ -380,6 +380,9 @@ void ASTWorker::update(
   FileName);
   return;
 }
+// Remove the old AST if it's still in cache.
+IdleASTs.take(this);
+
 // Build the AST for diagnostics.
 llvm::Optional AST =
 buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);


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


[PATCH] D49991: [clangd] Do not build AST if no diagnostics were requested

2018-07-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ioeric.
Herald added subscribers: arphaman, jkorous, MaskRay, javed.absar.

It can be washed out from the cache before first access anyway, so
building it can just be a waste of time.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49991

Files:
  clangd/TUScheduler.cpp


Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -383,6 +383,14 @@
 // Remove the old AST if it's still in cache.
 IdleASTs.take(this);
 
+if (WantDiags == WantDiagnostics::No) {
+  // If no diagnostics were requested, building it in advance might be a
+  // waste if it will be removed from the cache before it is first 
accessed.
+  // So we choose to avoid building it in the first place, the rebuild will
+  // happen on first access instead.
+  return;
+}
+
 // Build the AST for diagnostics.
 llvm::Optional AST =
 buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);


Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -383,6 +383,14 @@
 // Remove the old AST if it's still in cache.
 IdleASTs.take(this);
 
+if (WantDiags == WantDiagnostics::No) {
+  // If no diagnostics were requested, building it in advance might be a
+  // waste if it will be removed from the cache before it is first accessed.
+  // So we choose to avoid building it in the first place, the rebuild will
+  // happen on first access instead.
+  return;
+}
+
 // Build the AST for diagnostics.
 llvm::Optional AST =
 buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-07-30 Thread Reka Kovacs via cfe-commits
Author: rkovacs
Date: Mon Jul 30 08:43:45 2018
New Revision: 338259

URL: http://llvm.org/viewvc/llvm-project?rev=338259&view=rev
Log:
[analyzer] Add support for more invalidating functions in InnerPointerChecker.

According to the standard, pointers referring to the elements of a
`basic_string` may be invalidated if they are used as an argument to
any standard library function taking a reference to non-const
`basic_string` as an argument. This patch makes InnerPointerChecker warn
for these cases.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/inner-pointer.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=338259&r1=338258&r2=338259&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Mon Jul 30 
08:43:45 2018
@@ -91,37 +91,53 @@ public:
 ReserveFn("reserve"), ResizeFn("resize"),
 ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
 
-  /// Check whether the function called on the container object is a
-  /// member function that potentially invalidates pointers referring
-  /// to the objects's internal buffer.
-  bool mayInvalidateBuffer(const CallEvent &Call) const;
-
-  /// Record the connection between the symbol returned by c_str() and the
-  /// corresponding string object region in the ProgramState. Mark the symbol
-  /// released if the string object is destroyed.
+  /// Check if the object of this member function call is a `basic_string`.
+  bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
+
+  /// Check whether the called member function potentially invalidates
+  /// pointers referring to the container object's inner buffer.
+  bool isInvalidatingMemberFunction(const CallEvent &Call) const;
+
+  /// Mark pointer symbols associated with the given memory region released
+  /// in the program state.
+  void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
+  const MemRegion *ObjRegion,
+  CheckerContext &C) const;
+
+  /// Standard library functions that take a non-const `basic_string` argument 
by
+  /// reference may invalidate its inner pointers. Check for these cases and
+  /// mark the pointers released.
+  void checkFunctionArguments(const CallEvent &Call, ProgramStateRef State,
+  CheckerContext &C) const;
+
+  /// Record the connection between raw pointers referring to a container
+  /// object's inner buffer and the object's memory region in the program 
state.
+  /// Mark potentially invalidated pointers released.
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
-  /// Clean up the ProgramState map.
+  /// Clean up the program state map.
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 };
 
 } // end anonymous namespace
 
-// [string.require]
-//
-// "References, pointers, and iterators referring to the elements of a
-// basic_string sequence may be invalidated by the following uses of that
-// basic_string object:
-//
-// -- TODO: As an argument to any standard library function taking a reference
-// to non-const basic_string as an argument. For example, as an argument to
-// non-member functions swap(), operator>>(), and getline(), or as an argument
-// to basic_string::swap().
-//
-// -- Calling non-const member functions, except operator[], at, front, back,
-// begin, rbegin, end, and rend."
-//
-bool InnerPointerChecker::mayInvalidateBuffer(const CallEvent &Call) const {
+bool InnerPointerChecker::isCalledOnStringObject(
+const CXXInstanceCall *ICall) const {
+  const auto *ObjRegion =
+dyn_cast_or_null(ICall->getCXXThisVal().getAsRegion());
+  if (!ObjRegion)
+return false;
+
+  QualType ObjTy = ObjRegion->getValueType();
+  if (ObjTy.isNull() ||
+  ObjTy->getAsCXXRecordDecl()->getName() != "basic_string")
+return false;
+
+  return true;
+}
+
+bool InnerPointerChecker::isInvalidatingMemberFunction(
+const CallEvent &Call) const {
   if (const auto *MemOpCall = dyn_cast(&Call)) {
 OverloadedOperatorKind Opc = MemOpCall->getOriginExpr()->getOperator();
 if (Opc == OO_Equal || Opc == OO_PlusEqual)
@@ -137,53 +153,104 @@ bool InnerPointerChecker::mayInvalidateB
   Call.isCalled(SwapFn));
 }
 
-void InnerPointerChecker::checkPostCall(const CallEvent &Call,
-CheckerContext &C) const {
-  const auto *ICall = dyn_cast(&Call);
-  if (!ICall)
-return;
-
-  SVal Obj = ICall->getCXXThisVal();
-  const auto *ObjRegion = 
dyn_cast_or_null(Obj.

[PATCH] D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker

2018-07-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338259: [analyzer] Add support for more invalidating 
functions in InnerPointerChecker. (authored by rkovacs, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D49656

Files:
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/inner-pointer.cpp

Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -38,13 +38,29 @@
 typedef basic_string u16string;
 typedef basic_string u32string;
 
+template 
+void func_ref(T &a);
+
+template 
+void func_const_ref(const T &a);
+
+template 
+void func_value(T a);
+
+string my_string = "default";
+void default_arg(int a = 42, string &b = my_string);
+
 } // end namespace std
 
 void consume(const char *) {}
 void consume(const wchar_t *) {}
 void consume(const char16_t *) {}
 void consume(const char32_t *) {}
 
+//=--=//
+// `std::string` member functions //
+//=--=//
+
 void deref_after_scope_char(bool cond) {
   const char *c, *d;
   {
@@ -151,6 +167,19 @@
   }  // expected-note@-1 {{Use of memory after it is freed}}
 }
 
+void deref_after_scope_ok(bool cond) {
+  const char *c, *d;
+  std::string s;
+  {
+c = s.c_str();
+d = s.data();
+  }
+  if (cond)
+consume(c); // no-warning
+  else
+consume(d); // no-warning
+}
+
 void deref_after_equals() {
   const char *c;
   std::string s = "hello";
@@ -277,15 +306,58 @@
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
-void deref_after_scope_ok(bool cond) {
-  const char *c, *d;
+//=---=//
+// Other STL functions //
+//=---=//
+
+void STL_func_ref() {
+  const char *c;
   std::string s;
-  {
-c = s.c_str();
-d = s.data();
-  }
-  if (cond)
-consume(c); // no-warning
-  else
-consume(d); // no-warning
+  c = s.c_str();// expected-note {{Dangling inner pointer obtained here}}
+  std::func_ref(s); // expected-note {{Inner pointer invalidated by call to 'func_ref'}}
+  consume(c);   // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void STL_func_const_ref() {
+  const char *c;
+  std::string s;
+  c = s.c_str();
+  std::func_const_ref(s);
+  consume(c); // no-warning
+}
+
+void STL_func_value() {
+  const char *c;
+  std::string s;
+  c = s.c_str();
+  std::func_value(s);
+  consume(c); // no-warning
+}
+
+void func_ptr_known() {
+  const char *c;
+  std::string s;
+  void (*func_ptr)(std::string &) = std::func_ref;
+  c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+  func_ptr(s);   // expected-note {{Inner pointer invalidated by call to 'func_ref'}}
+  consume(c);// expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void func_ptr_unknown(void (*func_ptr)(std::string &)) {
+  const char *c;
+  std::string s;
+  c = s.c_str();
+  func_ptr(s);
+  consume(c); // no-warning
+}
+
+void func_default_arg() {
+  const char *c;
+  std::string s;
+  c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+  default_arg(3, s); // expected-note {{Inner pointer invalidated by call to 'default_arg'}}
+  consume(c);// expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
 }
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -91,37 +91,53 @@
 ReserveFn("reserve"), ResizeFn("resize"),
 ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
 
-  /// Check whether the function called on the container object is a
-  /// member function that potentially invalidates pointers referring
-  /// to the objects's internal buffer.
-  bool mayInvalidateBuffer(const CallEvent &Call) const;
-
-  /// Record the connection between the symbol returned by c_str() and the
-  /// corresponding string object region in the ProgramState. Mark the symbol
-  /// released if the string object is destroyed.
+  /// Check if the object of this member function call is a `basic_string`.
+  bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
+
+  /// Check whether the called member function potentially invalidates
+  /// pointers referring to the container object's inner buffer.
+  bool isInvalidatingMemberFunction(const CallEvent &Call) const;
+
+  /// Mark pointer symbols associated with the given memory region released
+  /// in the program state.
+  void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef Stat

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In https://reviews.llvm.org/D49317#1180200, @mclow.lists wrote:

> I am not in favor of this patch.
>
> I'm in favor of fixing the problem that Arthur has described, but not like 
> this, for the following reasons:
>
> - Conceptually, these are (similar to) "Allocator-based versions of the 
> algorithms proposed in P0040 ", and should 
> (probably? possibly?) look more like them.
> - Mainly, though, I think that the goal of this patch (which is see as 
> 'getting to memcpy') is not the direction that libc++ should take.  Instead, 
> we should be writing simple loops that the compiler can optimize into a call 
> to memcpy if it chooses. Having calls to `memcpy` in the code paths makes it 
> impossible to "constexp-ify" this code. (See 
> https://libcxx.llvm.org/cxx2a_status.html (comments on `std::copy` and 
> https://bugs.llvm.org/show_bug.cgi?id=25165).


Marshall makes a great point about `memcpy` and `constexpr`... We're trying to 
make the default allocator constexpr-friendly for C++20, and this doesn't play 
very nicely with that.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

2018-07-30 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+  continue;
+
+// If the CurrentObject is a field of OtherObject, it will be analyzed
+// during the analysis of OtherObject.

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > It seems safer to look at `CXXConstructExpr::getConstructionKind()`.
> > > 
> > > Taking a `LazyCompoundVal` and converting it back to a region is 
> > > definitely a bad idea because the region within `LazyCompoundVal` is 
> > > completely immaterial and carries no meaning for the value represented by 
> > > this `SVal`; it's not necessarily the region you're looking for.
> > Looking at the singleton test case, I think I need to get that region 
> > somehow, and I'm not too sure what you meant under using 
> > `CXXConstructExpr::getConstructionKind()`. One thing I could do, is similar 
> > to how `getObjectVal` works:
> > ```
> >   Loc Tmp = Context.getSValBuilder().getCXXThis(OtherCtor->getParent(),
> > Context.getStackFrame());
> > 
> >   auto OtherThis = 
> > Context.getState()->getSVal(Tmp).castAs();
> > 
> >   OtherThis.getRegion(); // Hooray!
> > ```
> > 
> > I have tested it, and it works wonders. Is this a "safe-to-use" region?
> You can directly ask `CXXConstructExpr` if it's a field or a base 
> constructor. I could describe `getConstructionKind()` as some sort of very 
> limited `ConstructionContext` that's available in the AST: it explains to you 
> what sort of object is being constructed. I suspect it's enough for your 
> purposes.
I think I'm aware of how it works, but my issue is that it's very limited:
```
enum ConstructionKind { CK_Complete, CK_NonVirtualBase, CK_VirtualBase, 
CK_Delegating }
```
Sadly, whether the `CXXConstructExpr` was a field construction or a "top level" 
(non-field) construction can't be retrieved from `getConstructionKind()`, as I 
understand it.

If I were to return true (say that the constructed object will be analyzed 
later, thus ignoring it for now) if `getConstructionKind() == CK_Complete`, the 
singleton testcase will fail (I have tested it). I'm very confident that I need 
to obtain the constructed object's `MemRegion`.


https://reviews.llvm.org/D48436



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


[clang-tools-extra] r338260 - [clangd] Remove outdated comment. NFC

2018-07-30 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Mon Jul 30 08:55:13 2018
New Revision: 338260

URL: http://llvm.org/viewvc/llvm-project?rev=338260&view=rev
Log:
[clangd] Remove outdated comment. NFC

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=338260&r1=338259&r2=338260&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Jul 30 08:55:13 2018
@@ -107,8 +107,6 @@ public:
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
   /// separate thread. When the parsing is complete, DiagConsumer passed in
   /// constructor will receive onDiagnosticsReady callback.
-  /// When \p SkipCache is true, compile commands will always be requested from
-  /// compilation database even if they were cached in previous invocations.
   void addDocument(PathRef File, StringRef Contents,
WantDiagnostics WD = WantDiagnostics::Auto);
 


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


[PATCH] D49897: [WebAssembly] Force use of lld for test/Driver/wasm-toolchain.c(pp)

2018-07-30 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

How do you configure clang to use a different linker?  It looks like 
getDefaultLinker() is hardcoded, but maybe I'm missing something.


Repository:
  rC Clang

https://reviews.llvm.org/D49897



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


r338263 - [analyzer] Add missing state transition in IteratorChecker.

2018-07-30 Thread Reka Kovacs via cfe-commits
Author: rkovacs
Date: Mon Jul 30 09:14:59 2018
New Revision: 338263

URL: http://llvm.org/viewvc/llvm-project?rev=338263&view=rev
Log:
[analyzer] Add missing state transition in IteratorChecker.

After cleaning up program state maps in `checkDeadSymbols()`,
a transition should be added to generate the new state.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=338263&r1=338262&r2=338263&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Mon Jul 30 
09:14:59 2018
@@ -551,6 +551,8 @@ void IteratorChecker::checkDeadSymbols(S
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,


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


[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

2018-07-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338263: [analyzer] Add missing state transition in 
IteratorChecker. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47417?vs=148735&id=157983#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47417

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -551,6 +551,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -551,6 +551,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

2018-07-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338263: [analyzer] Add missing state transition in 
IteratorChecker. (authored by rkovacs, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D47417

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -551,6 +551,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -551,6 +551,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

2018-07-30 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

Thank you for taking a look @yaxunl. Should I wait for another reviewer or can 
I commit this?


Repository:
  rC Clang

https://reviews.llvm.org/D49930



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


[PATCH] D49986: [ADT] ImmutableList::add parameters are switched

2018-07-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment.

I'm a bit confused: does it mean these methods are never called in Clang?


Repository:
  rC Clang

https://reviews.llvm.org/D49986



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


[PATCH] D49058: [analyzer] Move InnerPointerChecker out of alpha

2018-07-30 Thread Reka Kovacs via Phabricator via cfe-commits
rnkovacs updated this revision to Diff 157987.
rnkovacs retitled this revision from "[analyzer] Move 
DanglingInternalBufferChecker out of alpha" to "[analyzer] Move 
InnerPointerChecker out of alpha".
rnkovacs added a comment.

Rebase.


https://reviews.llvm.org/D49058

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  test/Analysis/inner-pointer.cpp


Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -1,4 +1,4 @@
-//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.InnerPointer %s 
-analyzer-output=text -verify
+//RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer %s 
-analyzer-output=text -verify
 
 namespace std {
 
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -276,6 +276,10 @@
 
 let ParentPackage = Cplusplus in {
 
+def InnerPointerChecker : Checker<"InnerPointer">,
+  HelpText<"Check for inner pointers of C++ containers used after 
re/deallocation">,
+  DescFile<"InnerPointerChecker.cpp">;
+
 def NewDeleteChecker : Checker<"NewDelete">,
   HelpText<"Check for double-free and use-after-free problems. Traces memory 
managed by new/delete.">,
   DescFile<"MallocChecker.cpp">;
@@ -305,10 +309,6 @@
"destructor in their base class">,
   DescFile<"DeleteWithNonVirtualDtorChecker.cpp">;
 
-def InnerPointerChecker : Checker<"InnerPointer">,
-  HelpText<"Check for inner pointers of C++ containers used after 
re/deallocation">,
-  DescFile<"InnerPointerChecker.cpp">;
-
 def IteratorRangeChecker : Checker<"IteratorRange">,
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;


Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -1,4 +1,4 @@
-//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.InnerPointer %s -analyzer-output=text -verify
+//RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer %s -analyzer-output=text -verify
 
 namespace std {
 
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -276,6 +276,10 @@
 
 let ParentPackage = Cplusplus in {
 
+def InnerPointerChecker : Checker<"InnerPointer">,
+  HelpText<"Check for inner pointers of C++ containers used after re/deallocation">,
+  DescFile<"InnerPointerChecker.cpp">;
+
 def NewDeleteChecker : Checker<"NewDelete">,
   HelpText<"Check for double-free and use-after-free problems. Traces memory managed by new/delete.">,
   DescFile<"MallocChecker.cpp">;
@@ -305,10 +309,6 @@
"destructor in their base class">,
   DescFile<"DeleteWithNonVirtualDtorChecker.cpp">;
 
-def InnerPointerChecker : Checker<"InnerPointer">,
-  HelpText<"Check for inner pointers of C++ containers used after re/deallocation">,
-  DescFile<"InnerPointerChecker.cpp">;
-
 def IteratorRangeChecker : Checker<"IteratorRange">,
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49058: [analyzer] Move InnerPointerChecker out of alpha

2018-07-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

Ok, cool! Let's get this done.


https://reviews.llvm.org/D49058



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


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: include/vector:318
+}
+}
+

Marshall writes:
> Instead, we should be writing simple loops that the compiler can optimize 
> into a call to memcpy if it chooses. Having calls to memcpy in the code paths 
> makes it impossible to "constexp-ify" this code.

Well, I have three thoughts on that.

(A), "removing the calls to memcpy" sounds like you want to just call the 
*actual* move-constructor in a loop, and then later call the actual destructor 
in a loop. Which is to say, you don't want libc++ to have a codepath for this 
speed optimization at all. That's just leaving a ton of performance on the 
table, and I strongly disagree with that idea.

(B), regardless, couldn't you achieve that goal simply by taking this patch 
almost exactly as it is except removing the overloads that take `true_type`? If 
you want constexpr-friendliness badly enough that you're willing to call the 
move-constructor and destructor even of trivially copyable types, then you can 
still use this framework; you just have to remove the overloads that call 
memcpy. That wouldn't be a major refactoring.

(C), surely if you want the best of both worlds, you should be pushing someone 
to invent a constexpr memcpy and/or a way to [detect constexpr-context at 
compile 
time](https://quuxplusone.github.io/blog/2018/06/12/perennial-impossibilities/#detect-the-constexprness-of-the-current-context)?
 I don't think it makes sense to pessimize existing (non-constexpr) users in 
C++03-through-C++17 just because someone hypothetically might in C++2a-or-later 
want to mutate a std::vector in a constexpr context.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

2018-07-30 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

Is it possible/reasonable for the test to call __llvm_profile_recursive_mkdir 
and then verify the mode of the created directory?
The test would have to be flagged as unsupported on Windows but that seems fine.


https://reviews.llvm.org/D49953



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


[PATCH] D48794: [AArch64] Implement execute-only support.

2018-07-30 Thread Ivan Lozano via Phabricator via cfe-commits
ivanlozano abandoned this revision.
ivanlozano added a comment.

Alternative solution committed: https://reviews.llvm.org/rL338271


Repository:
  rC Clang

https://reviews.llvm.org/D48794



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


[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

2018-07-30 Thread Matt Davis via Phabricator via cfe-commits
mattd added a comment.

In https://reviews.llvm.org/D49953#1180466, @probinson wrote:

> Is it possible/reasonable for the test to call __llvm_profile_recursive_mkdir 
> and then verify the mode of the created directory?
>  The test would have to be flagged as unsupported on Windows but that seems 
> fine.


That't totally possible, as long as it's fine to call fstat()/stat() to gather 
the mode value on the resulting directory.   We'd also
need to get the umask() so that we can ensure that the correct value is set 
with respect to the user's umask.


https://reviews.llvm.org/D49953



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


[PATCH] D49916: [CodeGen] Add to emitted DebugLoc information about coverage when it's required

2018-07-30 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Test coverage?


Repository:
  rC Clang

https://reviews.llvm.org/D49916



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


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments.



Comment at: include/vector:318
+}
+}
+

Quuxplusone wrote:
> Marshall writes:
> > Instead, we should be writing simple loops that the compiler can optimize 
> > into a call to memcpy if it chooses. Having calls to memcpy in the code 
> > paths makes it impossible to "constexp-ify" this code.
> 
> Well, I have three thoughts on that.
> 
> (A), "removing the calls to memcpy" sounds like you want to just call the 
> *actual* move-constructor in a loop, and then later call the actual 
> destructor in a loop. Which is to say, you don't want libc++ to have a 
> codepath for this speed optimization at all. That's just leaving a ton of 
> performance on the table, and I strongly disagree with that idea.
> 
> (B), regardless, couldn't you achieve that goal simply by taking this patch 
> almost exactly as it is except removing the overloads that take `true_type`? 
> If you want constexpr-friendliness badly enough that you're willing to call 
> the move-constructor and destructor even of trivially copyable types, then 
> you can still use this framework; you just have to remove the overloads that 
> call memcpy. That wouldn't be a major refactoring.
> 
> (C), surely if you want the best of both worlds, you should be pushing 
> someone to invent a constexpr memcpy and/or a way to [detect 
> constexpr-context at compile 
> time](https://quuxplusone.github.io/blog/2018/06/12/perennial-impossibilities/#detect-the-constexprness-of-the-current-context)?
>  I don't think it makes sense to pessimize existing (non-constexpr) users in 
> C++03-through-C++17 just because someone hypothetically might in 
> C++2a-or-later want to mutate a std::vector in a constexpr context.
> Which is to say, you don't want libc++ to have a codepath for this speed 
> optimization at all. 

You're completely correct. I don't want libc++ to have such a code path.
I want clang to generate a `memcpy` from the code w/o ever mentioning `memcpy` 
in the source.




Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: include/vector:318
+}
+}
+

mclow.lists wrote:
> Quuxplusone wrote:
> > Marshall writes:
> > > Instead, we should be writing simple loops that the compiler can optimize 
> > > into a call to memcpy if it chooses. Having calls to memcpy in the code 
> > > paths makes it impossible to "constexp-ify" this code.
> > 
> > Well, I have three thoughts on that.
> > 
> > (A), "removing the calls to memcpy" sounds like you want to just call the 
> > *actual* move-constructor in a loop, and then later call the actual 
> > destructor in a loop. Which is to say, you don't want libc++ to have a 
> > codepath for this speed optimization at all. That's just leaving a ton of 
> > performance on the table, and I strongly disagree with that idea.
> > 
> > (B), regardless, couldn't you achieve that goal simply by taking this patch 
> > almost exactly as it is except removing the overloads that take 
> > `true_type`? If you want constexpr-friendliness badly enough that you're 
> > willing to call the move-constructor and destructor even of trivially 
> > copyable types, then you can still use this framework; you just have to 
> > remove the overloads that call memcpy. That wouldn't be a major refactoring.
> > 
> > (C), surely if you want the best of both worlds, you should be pushing 
> > someone to invent a constexpr memcpy and/or a way to [detect 
> > constexpr-context at compile 
> > time](https://quuxplusone.github.io/blog/2018/06/12/perennial-impossibilities/#detect-the-constexprness-of-the-current-context)?
> >  I don't think it makes sense to pessimize existing (non-constexpr) users 
> > in C++03-through-C++17 just because someone hypothetically might in 
> > C++2a-or-later want to mutate a std::vector in a constexpr context.
> > Which is to say, you don't want libc++ to have a codepath for this speed 
> > optimization at all. 
> 
> You're completely correct. I don't want libc++ to have such a code path.
> I want clang to generate a `memcpy` from the code w/o ever mentioning 
> `memcpy` in the source.
> 
> 
@mclow.lists: So would you accept a version of this patch that simply removed 
the `true_type` overloads? That would change this from a pure refactoring to a 
performance regression, but it would still reduce the overall diff between 
libc++ master and libc++ trivially-relocatable.

(Maybe it's no longer clear and needs restating: This patch is //currently// a 
pure refactoring. All I'm doing is moving the //existing// helper functions out 
of allocator_traits. IIUC, your objections apply to the existence of these 
//existing// helper functions just as much as to the refactored versions.)


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


r338279 - [ARM, AArch64]: Use unadjusted alignment when passing composites as arguments

2018-07-30 Thread Momchil Velikov via cfe-commits
Author: chill
Date: Mon Jul 30 10:48:23 2018
New Revision: 338279

URL: http://llvm.org/viewvc/llvm-project?rev=338279&view=rev
Log:
[ARM, AArch64]: Use unadjusted alignment when passing composites as arguments

The "Procedure Call Procedure Call Standard for the ARM® Architecture"
(https://static.docs.arm.com/ihi0042/f/IHI0042F_aapcs.pdf), specifies that
composite types are passed according to their "natural alignment", i.e. the
alignment before alignment adjustment on the entire composite is applied.

The same applies for AArch64 ABI.

Clang, however, used the adjusted alignment.

GCC already implements the ABI correctly. With this patch Clang becomes
compatible with GCC and passes such arguments in accordance with AAPCS.

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


Added:
cfe/trunk/test/CodeGen/aapcs-align.cc
cfe/trunk/test/CodeGen/aapcs64-align.cc
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/RecordLayout.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/RecordLayout.cpp
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGen/arm-arguments.c

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=338279&r1=338278&r2=338279&view=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Mon Jul 30 10:48:23 2018
@@ -226,6 +226,12 @@ class ASTContext : public RefCountedBase
   using TypeInfoMap = llvm::DenseMap;
   mutable TypeInfoMap MemoizedTypeInfo;
 
+  /// A cache from types to unadjusted alignment information. Only ARM and
+  /// AArch64 targets need this information, keeping it separate prevents
+  /// imposing overhead on TypeInfo size.
+  using UnadjustedAlignMap = llvm::DenseMap;
+  mutable UnadjustedAlignMap MemoizedUnadjustedAlign;
+
   /// A cache mapping from CXXRecordDecls to key functions.
   llvm::DenseMap KeyFunctions;
 
@@ -2067,6 +2073,16 @@ public:
   unsigned getTypeAlign(QualType T) const { return getTypeInfo(T).Align; }
   unsigned getTypeAlign(const Type *T) const { return getTypeInfo(T).Align; }
 
+  /// Return the ABI-specified natural alignment of a (complete) type \p T,
+  /// before alignment adjustments, in bits.
+  ///
+  /// This alignment is curently used only by ARM and AArch64 when passing
+  /// arguments of a composite type.
+  unsigned getTypeUnadjustedAlign(QualType T) const {
+return getTypeUnadjustedAlign(T.getTypePtr());
+  }
+  unsigned getTypeUnadjustedAlign(const Type *T) const;
+
   /// Return the ABI-specified alignment of a type, in bits, or 0 if
   /// the type is incomplete and we cannot determine the alignment (for
   /// example, from alignment attributes).
@@ -2077,6 +2093,12 @@ public:
   CharUnits getTypeAlignInChars(QualType T) const;
   CharUnits getTypeAlignInChars(const Type *T) const;
 
+  /// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a 
type,
+  /// in characters, before alignment adjustments. This method does not work on
+  /// incomplete types.
+  CharUnits getTypeUnadjustedAlignInChars(QualType T) const;
+  CharUnits getTypeUnadjustedAlignInChars(const Type *T) const;
+
   // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the
   // type is a record, its data size is returned.
   std::pair getTypeInfoDataSizeInChars(QualType T) const;

Modified: cfe/trunk/include/clang/AST/RecordLayout.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecordLayout.h?rev=338279&r1=338278&r2=338279&view=diff
==
--- cfe/trunk/include/clang/AST/RecordLayout.h (original)
+++ cfe/trunk/include/clang/AST/RecordLayout.h Mon Jul 30 10:48:23 2018
@@ -71,6 +71,10 @@ private:
   // Alignment - Alignment of record in characters.
   CharUnits Alignment;
 
+  // UnadjustedAlignment - Maximum of the alignments of the record members in
+  // characters.
+  CharUnits UnadjustedAlignment;
+
   /// RequiredAlignment - The required alignment of the object.  In the MS-ABI
   /// the __declspec(align()) trumps #pramga pack and must always be obeyed.
   CharUnits RequiredAlignment;
@@ -136,6 +140,7 @@ private:
   CXXRecordLayoutInfo *CXXInfo = nullptr;
 
   ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment,
+  CharUnits unadjustedAlignment,
   CharUnits requiredAlignment, CharUnits datasize,
   ArrayRef fieldoffsets);
 
@@ -144,6 +149,7 @@ private:
   // Constructor for C++ records.
   ASTRecordLayout(const ASTContext &Ctx,
   CharUnits size, CharUnits alignment,
+  CharUnits unadjustedAlignment,
   CharUnits requiredAlignment,
   bool hasOwnVFPtr, bool hasExtendabl

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

2018-07-30 Thread Momchil Velikov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338279: [ARM, AArch64]: Use unadjusted alignment when 
passing composites as arguments (authored by chill, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46013

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/RecordLayout.h
  lib/AST/ASTContext.cpp
  lib/AST/RecordLayout.cpp
  lib/AST/RecordLayoutBuilder.cpp
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/aapcs-align.cc
  test/CodeGen/aapcs64-align.cc
  test/CodeGen/arm-arguments.c

Index: include/clang/AST/RecordLayout.h
===
--- include/clang/AST/RecordLayout.h
+++ include/clang/AST/RecordLayout.h
@@ -71,6 +71,10 @@
   // Alignment - Alignment of record in characters.
   CharUnits Alignment;
 
+  // UnadjustedAlignment - Maximum of the alignments of the record members in
+  // characters.
+  CharUnits UnadjustedAlignment;
+
   /// RequiredAlignment - The required alignment of the object.  In the MS-ABI
   /// the __declspec(align()) trumps #pramga pack and must always be obeyed.
   CharUnits RequiredAlignment;
@@ -136,14 +140,16 @@
   CXXRecordLayoutInfo *CXXInfo = nullptr;
 
   ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment,
+  CharUnits unadjustedAlignment,
   CharUnits requiredAlignment, CharUnits datasize,
   ArrayRef fieldoffsets);
 
   using BaseOffsetsMapTy = CXXRecordLayoutInfo::BaseOffsetsMapTy;
 
   // Constructor for C++ records.
   ASTRecordLayout(const ASTContext &Ctx,
   CharUnits size, CharUnits alignment,
+  CharUnits unadjustedAlignment,
   CharUnits requiredAlignment,
   bool hasOwnVFPtr, bool hasExtendableVFPtr,
   CharUnits vbptroffset,
@@ -170,6 +176,10 @@
   /// getAlignment - Get the record alignment in characters.
   CharUnits getAlignment() const { return Alignment; }
 
+  /// getUnadjustedAlignment - Get the record alignment in characters, before
+  /// alignment adjustement.
+  CharUnits getUnadjustedAlignment() const { return UnadjustedAlignment; }
+
   /// getSize - Get the record size in characters.
   CharUnits getSize() const { return Size; }
 
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -226,6 +226,12 @@
   using TypeInfoMap = llvm::DenseMap;
   mutable TypeInfoMap MemoizedTypeInfo;
 
+  /// A cache from types to unadjusted alignment information. Only ARM and
+  /// AArch64 targets need this information, keeping it separate prevents
+  /// imposing overhead on TypeInfo size.
+  using UnadjustedAlignMap = llvm::DenseMap;
+  mutable UnadjustedAlignMap MemoizedUnadjustedAlign;
+
   /// A cache mapping from CXXRecordDecls to key functions.
   llvm::DenseMap KeyFunctions;
 
@@ -2067,6 +2073,16 @@
   unsigned getTypeAlign(QualType T) const { return getTypeInfo(T).Align; }
   unsigned getTypeAlign(const Type *T) const { return getTypeInfo(T).Align; }
 
+  /// Return the ABI-specified natural alignment of a (complete) type \p T,
+  /// before alignment adjustments, in bits.
+  ///
+  /// This alignment is curently used only by ARM and AArch64 when passing
+  /// arguments of a composite type.
+  unsigned getTypeUnadjustedAlign(QualType T) const {
+return getTypeUnadjustedAlign(T.getTypePtr());
+  }
+  unsigned getTypeUnadjustedAlign(const Type *T) const;
+
   /// Return the ABI-specified alignment of a type, in bits, or 0 if
   /// the type is incomplete and we cannot determine the alignment (for
   /// example, from alignment attributes).
@@ -2077,6 +2093,12 @@
   CharUnits getTypeAlignInChars(QualType T) const;
   CharUnits getTypeAlignInChars(const Type *T) const;
 
+  /// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a type,
+  /// in characters, before alignment adjustments. This method does not work on
+  /// incomplete types.
+  CharUnits getTypeUnadjustedAlignInChars(QualType T) const;
+  CharUnits getTypeUnadjustedAlignInChars(const Type *T) const;
+
   // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the
   // type is a record, its data size is returned.
   std::pair getTypeInfoDataSizeInChars(QualType T) const;
Index: test/CodeGen/arm-arguments.c
===
--- test/CodeGen/arm-arguments.c
+++ test/CodeGen/arm-arguments.c
@@ -211,10 +211,13 @@
 // APCS-GNU: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align {{[0-9]+}} %[[b]], i8* align {{[0-9]+}} %[[c]]
 // APCS-GNU: %[[d:.*]] = bitcast %struct.s35* %[[a]] to <4 x float>*
 // APCS-GNU: load <4 x float>, <4 x float>* %[[d]], align 16
-// AAPCS-LABEL: define arm_aapcscc <4 x float> @f35(i32 %i, %struct.s35* byval align 8, %struct.s35* byval align 8)
-// AAPCS: %[[a:.*]] = alloca %struct.s35, align 16
-/

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

2018-07-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.

Only comments on documentation and assertions. Feel free to commit once these 
are addressed to your satisfaction.




Comment at: docs/ReleaseNotes.rst:310
+
+  Just like ``-fsanitize=integer``, these issues are **not** undefined
+  behaviour. But they are not *always* intentional, and are somewhat hard to

"Just like -fsanitize=integer" -> "Just like other -fsanitize=integer checks", 
now that this is part of  `-fsanitize=integer`.



Comment at: docs/UndefinedBehaviorSanitizer.rst:17
 * Signed integer overflow
+* Problematic Implicit Casts (not UB, but not always intentional)
 * Conversion to, from, or between floating-point types which would

rsmith wrote:
> Don't use Title Caps here. "Problematic implicit conversions"
I don't think it makes sense to list this here, as it's not undefined behavior, 
and this is a list of undefined behavior that UBSan catches. (And I think it 
makes sense from a communication perspective to consider the non-UB checks to 
be "not part of UBSan but handled by the same infrastructure".)



Comment at: docs/UndefinedBehaviorSanitizer.rst:97
+ width, is not equal to the original value before the downcast.
+ This issue may often be caused by an implicit integer conversions.
   -  ``-fsanitize=integer-divide-by-zero``: Integer division by zero.

I don't think this last sentence adds anything, and it creates the impression 
that the issue is sometimes caused by something other than implicit integer 
conversions (which it isn't, as far as I can tell). Maybe just delete the last 
sentence here?

And instead, something like this would be useful:

"Issues caught by this sanitizer are not undefined behavior, but are often 
unintentional."



Comment at: docs/UndefinedBehaviorSanitizer.rst:136-137
+ overflow happens (signed or unsigned).
+ Both of these two issues are handled by ``-fsanitize=implicit-conversion``
+ group of checks.
   -  ``-fsanitize=unreachable``: If control flow reaches an unreachable

I don't think that's true (not until you add a sanitizer for signed <-> 
unsigned conversions that change the value). `4U / -2` produces the unexpected 
result `0U` rather than the mathematically-correct result `-2`, and 
`-fsanitize=implicit-conversion` doesn't catch it. Maybe just strike this 
sentence for now?

In fact... I think this is too much text to be adding to this bulleted list, 
which is just supposed to summarize the available checks. Maybe replace the 
description with

Signed integer overflow, where the result of a signed integer computation 
cannot be represented in its type. This includes all the checks covered by 
``-ftrapv``, as well as checks for signed division overflow (``INT_MIN/-1``), 
but not checks for lossy implicit conversions performed before the computation 
(see ``-fsanitize=implicit-conversion``).



Comment at: docs/UndefinedBehaviorSanitizer.rst:140-148
   -  ``-fsanitize=unsigned-integer-overflow``: Unsigned integer
  overflows. Note that unlike signed integer overflow, unsigned integer
  is not undefined behavior. However, while it has well-defined semantics,
- it is often unintentional, so UBSan offers to catch it.
+ it is often unintentional, so UBSan offers to catch it. Note that this
+ check does not diagnose lossy implicit integer conversions. Also note that
+ integer conversions may result in an unexpected computation result, even
+ though no overflow happens (signed or unsigned).

And here something like:

Unsigned integer overflow, where the result of an unsigned integer 
computation cannot be represented in its type. Unlike signed integer overflow, 
this is not undefined behavior, but it is often unintentional. This sanitizer 
does not check for lossy implicit conversions performed before such a 
computation (see ``-fsanitize=implicit-conversion``).



Comment at: docs/UndefinedBehaviorSanitizer.rst:165
  behavior (e.g. unsigned integer overflow).
+ Enables ``-fsanitize=implicit-integer-truncation``.
+  -  ``-fsanitize=implicit-conversion``: Checks for suspicious behaviours of

If we're going to list which sanitizers are enabled here, we should list them 
all:

Enables ``signed-integer-overflow``, ``unsigned-integer-overflow``, 
``shift``, ``integer-divide-by-zero``, and ``implicit-integer-truncation``.



Comment at: docs/UndefinedBehaviorSanitizer.rst:95
+ of bigger bit width to smaller bit width, if that results in data loss.
+ That is, if the demoted value, after casting back to the original width,
+ is not equal to the original value before the downcast. This issue may

lebedev.ri wrote:
> rsmith wrote:
> > rsmith wrote:
> > > erichkeane wrote:
> > > > I think the last 2 commas in t

r338282 - Delete some unreachable AST printing code.

2018-07-30 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Mon Jul 30 11:05:19 2018
New Revision: 338282

URL: http://llvm.org/viewvc/llvm-project?rev=338282&view=rev
Log:
Delete some unreachable AST printing code.

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

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=338282&r1=338281&r2=338282&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Mon Jul 30 11:05:19 2018
@@ -1489,36 +1489,6 @@ void TypePrinter::printAttributedAfter(c
 break;
   }
 
-  case AttributedType::attr_objc_gc: {
-OS << "objc_gc(";
-
-QualType tmp = T->getEquivalentType();
-while (tmp.getObjCGCAttr() == Qualifiers::GCNone) {
-  QualType next = tmp->getPointeeType();
-  if (next == tmp) break;
-  tmp = next;
-}
-
-if (tmp.isObjCGCWeak())
-  OS << "weak";
-else
-  OS << "strong";
-OS << ')';
-break;
-  }
-
-  case AttributedType::attr_objc_ownership:
-OS << "objc_ownership(";
-switch (T->getEquivalentType().getObjCLifetime()) {
-case Qualifiers::OCL_None: llvm_unreachable("no ownership!");
-case Qualifiers::OCL_ExplicitNone: OS << "none"; break;
-case Qualifiers::OCL_Strong: OS << "strong"; break;
-case Qualifiers::OCL_Weak: OS << "weak"; break;
-case Qualifiers::OCL_Autoreleasing: OS << "autoreleasing"; break;
-}
-OS << ')';
-break;
-
   case AttributedType::attr_ns_returns_retained:
 OS << "ns_returns_retained";
 break;


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


[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

2018-07-30 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 158017.
aheejin added a comment.
Herald added a subscriber: jfb.

- Changed the first argument of wake from i8* to i32*


Repository:
  rC Clang

https://reviews.llvm.org/D49396

Files:
  include/clang/Basic/BuiltinsWebAssembly.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins-wasm.c


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(int *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i32* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i32* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(long long *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(int *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i32* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i32* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12081,6 +12081,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i32);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i64: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i64);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wake: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *WakeCount = EmitScalarExpr(E->getArg(1));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wake);
+return Builder.CreateCall(Callee, {Addr, WakeCount});
+  }
 
   default:
 return nullptr;
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -34,4 +34,9 @@
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
 BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "UiLLi*LLiLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wake, "ULLii*LLi", "n")
+
 #undef BUILTIN


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(int *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i32* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i32* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(long long *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(int *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i32* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i32* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12081,6 +12081,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

> I don't think it makes sense to pessimize existing (non-constexpr) users in 
> C++03-through-C++17 just because someone hypothetically might in 
> C++2a-or-later want to mutate a std::vector in a constexpr context.

That's not the right (implied) question.

The correct question is:

> Will libc++pessimize existing (non-constexpr) users in C++03-through-C++17 
> ***who are using old compilers*** in order to support new constexpr features 
> that come down the pike?

And the answer to that is yes - eventually. 
I don't know when that will be, since the //new compilers// don't yet exist.
That's the point of https://bugs.llvm.org/show_bug.cgi?id=25165.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

@mclow.lists: Well, anyway, is this pure refactoring acceptable, or would you 
prefer to keep these helpers inside `allocator_traits` until a better solution 
to constexpr-memcpy can be invented?


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

2018-07-30 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 158024.
arphaman marked 6 inline comments as done.
arphaman added a comment.

Address review comments


https://reviews.llvm.org/D49758

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/tool/ClangdMain.cpp
  test/clangd/did-change-configuration-params.test

Index: test/clangd/did-change-configuration-params.test
===
--- /dev/null
+++ test/clangd/did-change-configuration-params.test
@@ -0,0 +1,51 @@
+# RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck -strict-whitespace %s
+# RUN: cat %t | FileCheck --check-prefix=ERR %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test", "compilationCommand": ["clang", "-c", "foo.c"]}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"}}}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///bar.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"}}}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [],
+# CHECK-NEXT:"uri": "file://{{.*}}/bar.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test2", "compilationCommand": ["clang", "-c", "foo.c", "-Wall", "-Werror"]}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 28,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 27,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 1
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+#
+# ERR: Updating file /clangd-test/foo.c with command [/clangd-test2] clang -c foo.c -Wall -Werror
+# Don't reparse the second file:
+# ERR: Skipping rebuild of the AST for /clangd-test/bar.c
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
+
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -166,6 +166,18 @@
 "eventually. Don't rely on it."),
 llvm::cl::init(""), llvm::cl::Hidden);
 
+enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
+
+static llvm::cl::opt CompileArgsFrom(
+"compile_args_from", llvm::cl::desc("The source of compile commands"),
+llvm::cl::values(clEnumValN(LSPCompileArgs, "lsp",
+"All compile commands come from LSP and "
+"'compile_commands.json' files are ignored"),
+ clEnumValN(FilesystemCompileArgs, "filesystem",
+"All compile commands come from the "
+"'compile_commands.json' files")),
+llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -278,7 +290,9 @@
   CCOpts.ShowOrigins = ShowOrigins;
 
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath, Opts);
+  ClangdLSPServer LSPServer(
+  Out, CCOpts, CompileCommandsDirPath,
+  /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   // Change stdin to binary to not lose \r\n on windows.
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -419,10 +419,24 @@
 };
 bool fromJSON(const llvm::json::Value &, DidChangeWatchedFilesParams &);
 
+/// Clangd extension that's used in the 'compilationDatabaseChanges' in
+//

[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

2018-07-30 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:170
+static llvm::cl::opt
+InMemoryCompileCommands("in-memory-compile-commands",
+llvm::cl::desc("Use an in-memory compile 
commands"),

ilya-biryukov wrote:
> The important distinction seems to be where the compile commands come from, 
> maybe make the name centered on the fact that compile commands are read from 
> the protocol rather from the filesystem?
> 
> E.g. using named option values:
> `--compile_args_from={lsp|filesystem}`
> Or a boolean option:
> `--compile_args_from_lsp`
> 
> WDYT?
Yeah, that looks better. I added this argument in the updated patch.


https://reviews.llvm.org/D49758



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


[PATCH] D49997: [libcxx] Fix _LIBCPP_NO_EXCEPTIONS redefined warning

2018-07-30 Thread Jason Lovett via Phabricator via cfe-commits
jasonl220 created this revision.
jasonl220 added a reviewer: mclow.lists.
Herald added a reviewer: EricWF.
Herald added subscribers: cfe-commits, ldionne.

Fix GCC 7.2.0 redefinition warning when LIBCXX_ENABLE_EXCEPTIONS cmake option 
is not set.

> llvm/projects/libcxx/include/__config:514:0: warning: "_LIBCPP_NO_EXCEPTIONS" 
> redefined
>  #define _LIBCPP_NO_EXCEPTIONS
> 
> :0:0: note: this is the location of the previous definition

This warning fires every time __config is included, 33 in total.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49997

Files:
  __config


Index: __config
===
--- __config
+++ __config
@@ -510,7 +510,7 @@
 #define _LIBCPP_HAS_IS_BASE_OF
 #endif
 
-#if !__EXCEPTIONS
+#if !__EXCEPTIONS && !defined(_LIBCPP_NO_EXCEPTIONS)
 #define _LIBCPP_NO_EXCEPTIONS
 #endif
 


Index: __config
===
--- __config
+++ __config
@@ -510,7 +510,7 @@
 #define _LIBCPP_HAS_IS_BASE_OF
 #endif
 
-#if !__EXCEPTIONS
+#if !__EXCEPTIONS && !defined(_LIBCPP_NO_EXCEPTIONS)
 #define _LIBCPP_NO_EXCEPTIONS
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker

2018-07-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2488
+
+  checkPreStmt(S, C);
+}

Let's do a common sub-function, similarly to how 
`MallocChecker::checkPointerEscape` and 
`MallocChecker::checkConstPointerEscape` both call 
`MallocChecker::checkPointerEscapeAux`. Should prevent us from unleashing 
`addTransition` hell if the code becomes more complicated.


https://reviews.llvm.org/D49361



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


[PATCH] D49999: [Hexagon] Remove fp-contract=fast setting for at O3

2018-07-30 Thread Brendon Cahoon via Phabricator via cfe-commits
bcahoon created this revision.
bcahoon added a reviewer: kparzysz.
Herald added a subscriber: cfe-commits.

Change Hexagon so that the setting for fp-contract is the default setting. This 
makes Hexagon consistent with all the other targets.


Repository:
  rC Clang

https://reviews.llvm.org/D4

Files:
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/test/Driver/hexagon-toolchain-elf.c


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -110,7 +110,7 @@
 // RUN:   -O3 \
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK026 %s
-// CHECK026: "-ffp-contract=fast"
+// CHECK026-NOT: "-ffp-contract=fast"
 // CHECK026: hexagon-link
 
 // RUN: %clang -### -target hexagon-unknown-elf \
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -513,11 +513,6 @@
 void HexagonToolChain::addClangTargetOptions(const ArgList &DriverArgs,
  ArgStringList &CC1Args,
  Action::OffloadKind) const {
-  if (!DriverArgs.hasArg(options::OPT_ffp_contract)) {
-unsigned OptLevel = getOptimizationLevel(DriverArgs);
-if (OptLevel >= 3)
-  CC1Args.push_back("-ffp-contract=fast");
-  }
   if (DriverArgs.hasArg(options::OPT_ffixed_r19)) {
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+reserved-r19");


Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -110,7 +110,7 @@
 // RUN:   -O3 \
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK026 %s
-// CHECK026: "-ffp-contract=fast"
+// CHECK026-NOT: "-ffp-contract=fast"
 // CHECK026: hexagon-link
 
 // RUN: %clang -### -target hexagon-unknown-elf \
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -513,11 +513,6 @@
 void HexagonToolChain::addClangTargetOptions(const ArgList &DriverArgs,
  ArgStringList &CC1Args,
  Action::OffloadKind) const {
-  if (!DriverArgs.hasArg(options::OPT_ffp_contract)) {
-unsigned OptLevel = getOptimizationLevel(DriverArgs);
-if (OptLevel >= 3)
-  CC1Args.push_back("-ffp-contract=fast");
-  }
   if (DriverArgs.hasArg(options::OPT_ffixed_r19)) {
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+reserved-r19");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-07-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:154
 
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE, bool 
Append) {
   if (CE->getNumArgs() != 3)

`Append` can be deduced from a call expression; I think it would be better to 
drop it from the parameter list and calculate again in the function body.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+  // - sizeof(dst)
+  if (Append && isSizeof(LenArg, DstArg))
+return true;

I am confused on what is happening in this branch and why is it bad. Could we 
add a commend?



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:196
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
-  return true;
+BufferLen -= DstOff;
+if (Append) {

Better to introduce a new variable, e.g. `RemainingBufferLen`


https://reviews.llvm.org/D49722



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


[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

2018-07-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 158030.
lebedev.ri marked 9 inline comments as done.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Address last portion of @rsmith review notes.

@rsmith, @vsk, @erichkeane - thank you for the review!


Repository:
  rC Clang

https://reviews.llvm.org/D48958

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  test/CodeGen/catch-implicit-integer-truncations.c
  test/CodeGenCXX/catch-implicit-integer-truncations.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,7 +29,22 @@
 // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent),?){5}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
+// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ???
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
 
 // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS
 // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}}
Index: test/CodeGenCXX/catch-implicit-integer-truncations.cpp
===
--- /dev/null
+++ test/CodeGenCXX/catch-implicit-integer-truncations.cpp
@@ -0,0 +1,256 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK
+// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fno-sanitize-recover=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER
+// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fsanitize-recover=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fsanitize=implicit-integer-truncation -fsanitize-trap=implicit-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP
+
+extern "C" { // Disable name mangling.
+
+// == //
+// Check that explicit cast does not interfere with implicit conversion
+// == //
+// These contain one implicit truncating conversion, and one explicit truncating cast.
+// We want to make sure that we still diagnose the implicit conversion.
+

[PATCH] D49998: [analyzer] Store ValueDecl in DeclRegion

2018-07-30 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338286: [analyzer] Store ValueDecl in DeclRegion (authored 
by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49998?vs=158027&id=158032#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49998

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h


Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -897,9 +897,9 @@
 
 class DeclRegion : public TypedValueRegion {
 protected:
-  const Decl *D;
+  const ValueDecl *D;
 
-  DeclRegion(const Decl *d, const MemRegion *sReg, Kind k)
+  DeclRegion(const ValueDecl *d, const MemRegion *sReg, Kind k)
   : TypedValueRegion(sReg, k), D(d) {
 assert(classof(this));
 assert(d);
@@ -909,7 +909,7 @@
   const MemRegion* superRegion, Kind k);
 
 public:
-  const Decl *getDecl() const { return D; }
+  const ValueDecl *getDecl() const { return D; }
   void Profile(llvm::FoldingSetNodeID& ID) const override;
 
   static bool classof(const MemRegion* R) {


Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -897,9 +897,9 @@
 
 class DeclRegion : public TypedValueRegion {
 protected:
-  const Decl *D;
+  const ValueDecl *D;
 
-  DeclRegion(const Decl *d, const MemRegion *sReg, Kind k)
+  DeclRegion(const ValueDecl *d, const MemRegion *sReg, Kind k)
   : TypedValueRegion(sReg, k), D(d) {
 assert(classof(this));
 assert(d);
@@ -909,7 +909,7 @@
   const MemRegion* superRegion, Kind k);
 
 public:
-  const Decl *getDecl() const { return D; }
+  const ValueDecl *getDecl() const { return D; }
   void Profile(llvm::FoldingSetNodeID& ID) const override;
 
   static bool classof(const MemRegion* R) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r338286 - [analyzer] Store ValueDecl in DeclRegion

2018-07-30 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Mon Jul 30 11:57:13 2018
New Revision: 338286

URL: http://llvm.org/viewvc/llvm-project?rev=338286&view=rev
Log:
[analyzer] Store ValueDecl in DeclRegion

All use cases of DeclRegion actually have ValueDecl there,
and getting the name from declaration comes in very handy.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=338286&r1=338285&r2=338286&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Mon 
Jul 30 11:57:13 2018
@@ -897,9 +897,9 @@ public:
 
 class DeclRegion : public TypedValueRegion {
 protected:
-  const Decl *D;
+  const ValueDecl *D;
 
-  DeclRegion(const Decl *d, const MemRegion *sReg, Kind k)
+  DeclRegion(const ValueDecl *d, const MemRegion *sReg, Kind k)
   : TypedValueRegion(sReg, k), D(d) {
 assert(classof(this));
 assert(d);
@@ -909,7 +909,7 @@ protected:
   const MemRegion* superRegion, Kind k);
 
 public:
-  const Decl *getDecl() const { return D; }
+  const ValueDecl *getDecl() const { return D; }
   void Profile(llvm::FoldingSetNodeID& ID) const override;
 
   static bool classof(const MemRegion* R) {


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


r338288 - [clang][ubsan] Implicit Conversion Sanitizer - integer truncation - clang part

2018-07-30 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Mon Jul 30 11:58:30 2018
New Revision: 338288

URL: http://llvm.org/viewvc/llvm-project?rev=338288&view=rev
Log:
[clang][ubsan] Implicit Conversion Sanitizer - integer truncation  - clang part

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

```
unsigned char store = 0;

bool consume(unsigned int val);

void test(unsigned long val) {
  if (consume(val)) {
// the 'val' is `unsigned long`, but `consume()` takes `unsigned int`.
// If their bit widths are different on this platform, the implicit
// truncation happens. And if that `unsigned long` had a value bigger
// than UINT_MAX, then you may or may not have a bug.

// Similarly, integer addition happens on `int`s, so `store` will
// be promoted to an `int`, the sum calculated (0+768=768),
// and the result demoted to `unsigned char`, and stored to `store`.
// In this case, the `store` will still be 0. Again, not always intended.
store = store + 768; // before addition, 'store' was promoted to int.
  }

  // But yes, sometimes this is intentional.
  // You can either make the conversion explicit
  (void)consume((unsigned int)val);
  // or mask the value so no bits will be *implicitly* lost.
  (void)consume((~((unsigned int)0)) & val);
}
```

Yes, there is a `-Wconversion`` diagnostic group, but first, it is kinda
noisy, since it warns on everything (unlike sanitizers, warning on an
actual issues), and second, there are cases where it does **not** warn.
So a Sanitizer is needed. I don't have any motivational numbers, but i know
i had this kind of problem 10-20 times, and it was never easy to track down.

The logic to detect whether an truncation has happened is pretty simple
if you think about it - https://godbolt.org/g/NEzXbb - basically, just
extend (using the new, not original!, signedness) the 'truncated' value
back to it's original width, and equality-compare it with the original value.

The most non-trivial thing here is the logic to detect whether this
`ImplicitCastExpr` AST node is **actually** an implicit conversion, //or//
part of an explicit cast. Because the explicit casts are modeled as an outer
`ExplicitCastExpr` with some `ImplicitCastExpr`'s as **direct** children.
https://godbolt.org/g/eE1GkJ

Nowadays, we can just use the new `part_of_explicit_cast` flag, which is set
on all the implicitly-added `ImplicitCastExpr`'s of an `ExplicitCastExpr`.
So if that flag is **not** set, then it is an actual implicit conversion.

As you may have noted, this isn't just named 
`-fsanitize=implicit-integer-truncation`.
There are potentially some more implicit conversions to be warned about.
Namely, implicit conversions that result in sign change; implicit conversion
between different floating point types, or between fp and an integer,
when again, that conversion is lossy.

One thing i know isn't handled is bitfields.

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

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=21530 | PR21530 ]], [[ 
https://bugs.llvm.org/show_bug.cgi?id=37552 | PR37552 ]], [[ 
https://bugs.llvm.org/show_bug.cgi?id=35409 | PR35409 ]].
Partially fixes [[ https://bugs.llvm.org/show_bug.cgi?id=9821 | PR9821 ]].
Fixes https://github.com/google/sanitizers/issues/940. (other than 
sign-changing implicit conversions)

Reviewers: rjmccall, rsmith, samsonov, pcc, vsk, eugenis, efriedma, kcc, 
erichkeane

Reviewed By: rsmith, vsk, erichkeane

Subscribers: erichkeane, klimek, #sanitizers, aaron.ballman, RKSimon, dtzWill, 
filcab, danielaustin, ygribov, dvyukov, milianw, mclow.lists, cfe-commits, 
regehr

Tags: #sanitizers

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

Added:
cfe/trunk/test/CodeGen/catch-implicit-integer-truncations.c
cfe/trunk/test/CodeGenCXX/catch-implicit-integer-truncations.cpp
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Basic/Sanitizers.h
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=338288&r1=338287&r2=338288&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Mon Jul 30 11:58:30 2018
@@ -46,7 +46,9 @@ sections with improvements to Clang's su
 Major New Features
 --
 
--  ...
+- A new Implicit Conversion Sanitizer (``-fsanitize=implicit-conversion``) 
group
+  was a

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

2018-07-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: docs/UndefinedBehaviorSanitizer.rst:136-137
+ overflow happens (signed or unsigned).
+ Both of these two issues are handled by ``-fsanitize=implicit-conversion``
+ group of checks.
   -  ``-fsanitize=unreachable``: If control flow reaches an unreachable

rsmith wrote:
> I don't think that's true (not until you add a sanitizer for signed <-> 
> unsigned conversions that change the value). `4U / -2` produces the 
> unexpected result `0U` rather than the mathematically-correct result `-2`, 
> and `-fsanitize=implicit-conversion` doesn't catch it. Maybe just strike this 
> sentence for now?
> 
> In fact... I think this is too much text to be adding to this bulleted list, 
> which is just supposed to summarize the available checks. Maybe replace the 
> description with
> 
> Signed integer overflow, where the result of a signed integer computation 
> cannot be represented in its type. This includes all the checks covered by 
> ``-ftrapv``, as well as checks for signed division overflow (``INT_MIN/-1``), 
> but not checks for lossy implicit conversions performed before the 
> computation (see ``-fsanitize=implicit-conversion``).
I will assume you meant "lossy implicit conversions performed *after* the 
computation".


Repository:
  rC Clang

https://reviews.llvm.org/D48958



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


[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

2018-07-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338288: [clang][ubsan] Implicit Conversion Sanitizer - 
integer truncation  - clang part (authored by lebedevri, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D48958

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  test/CodeGen/catch-implicit-integer-truncations.c
  test/CodeGenCXX/catch-implicit-integer-truncations.cpp
  test/Driver/fsanitize.c

Index: include/clang/Basic/Sanitizers.h
===
--- include/clang/Basic/Sanitizers.h
+++ include/clang/Basic/Sanitizers.h
@@ -84,7 +84,8 @@
 /// Return the sanitizers which do not affect preprocessing.
 inline SanitizerMask getPPTransparentSanitizers() {
   return SanitizerKind::CFI | SanitizerKind::Integer |
- SanitizerKind::Nullability | SanitizerKind::Undefined;
+ SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+ SanitizerKind::Undefined;
 }
 
 } // namespace clang
Index: include/clang/Basic/Sanitizers.def
===
--- include/clang/Basic/Sanitizers.def
+++ include/clang/Basic/Sanitizers.def
@@ -131,9 +131,14 @@
 // -fsanitize=undefined-trap is an alias for -fsanitize=undefined.
 SANITIZER_GROUP("undefined-trap", UndefinedTrap, Undefined)
 
+// ImplicitConversionSanitizer
+SANITIZER("implicit-integer-truncation", ImplicitIntegerTruncation)
+SANITIZER_GROUP("implicit-conversion", ImplicitConversion,
+ImplicitIntegerTruncation)
+
 SANITIZER_GROUP("integer", Integer,
-SignedIntegerOverflow | UnsignedIntegerOverflow | Shift |
-IntegerDivideByZero)
+ImplicitIntegerTruncation | IntegerDivideByZero | Shift |
+SignedIntegerOverflow | UnsignedIntegerOverflow)
 
 SANITIZER("local-bounds", LocalBounds)
 SANITIZER_GROUP("bounds", Bounds, ArrayBounds | LocalBounds)
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,7 +29,22 @@
 // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent),?){5}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
+// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ???
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
 
 // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS
 // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}}
Index: test/CodeGenCXX/cat

[PATCH] D49865: Inform the AST of #pragma FENV_ACCESS use

2018-07-30 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn updated this revision to Diff 158036.
kpn added a comment.

Updated based on review.


https://reviews.llvm.org/D49865

Files:
  include/clang/AST/Expr.h
  include/clang/Basic/LangOptions.h
  include/clang/Basic/TokenKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParsePragma.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaAttr.cpp

Index: lib/Sema/SemaAttr.cpp
===
--- lib/Sema/SemaAttr.cpp
+++ lib/Sema/SemaAttr.cpp
@@ -773,6 +773,18 @@
   }
 }
 
+void Sema::ActOnPragmaFEnvAccess(LangOptions::FEnvAccessModeKind FPC) {
+  switch (FPC) {
+  case LangOptions::FEA_On:
+FPFeatures.setAllowFEnvAccess();
+break;
+  case LangOptions::FEA_Off:
+FPFeatures.setDisallowFEnvAccess();
+break;
+  }
+}
+
+
 void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
SourceLocation Loc) {
   // Visibility calculations will consider the namespace's visibility.
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -674,6 +674,9 @@
   case tok::annot_pragma_fp_contract:
 HandlePragmaFPContract();
 return nullptr;
+  case tok::annot_pragma_fenv_access:
+HandlePragmaFEnvAccess();
+return nullptr;
   case tok::annot_pragma_fp:
 HandlePragmaFP();
 break;
Index: lib/Parse/ParseStmt.cpp
===
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -348,6 +348,11 @@
 ConsumeAnnotationToken();
 return StmtError();
 
+  case tok::annot_pragma_fenv_access:
+ProhibitAttributes(Attrs);
+HandlePragmaFEnvAccess();
+return StmtEmpty();
+
   case tok::annot_pragma_opencl_extension:
 ProhibitAttributes(Attrs);
 HandlePragmaOpenCLExtension();
@@ -914,6 +919,9 @@
 case tok::annot_pragma_fp:
   HandlePragmaFP();
   break;
+case tok::annot_pragma_fenv_access:
+  HandlePragmaFEnvAccess();
+  break;
 case tok::annot_pragma_ms_pointers_to_members:
   HandlePragmaMSPointersToMembers();
   break;
Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -106,8 +106,19 @@
 tok::OnOffSwitch OOS;
 if (PP.LexOnOffSwitch(OOS))
  return;
-if (OOS == tok::OOS_ON)
+if (OOS == tok::OOS_ON) {
   PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
+
+  MutableArrayRef Toks(PP.getPreprocessorAllocator().Allocate(1),
+1);
+  Toks[0].startToken();
+  Toks[0].setKind(tok::annot_pragma_fenv_access);
+  Toks[0].setLocation(Tok.getLocation());
+  Toks[0].setAnnotationEndLoc(Tok.getLocation());
+  Toks[0].setAnnotationValue(reinterpret_cast(
+   static_cast(OOS)));
+  PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
+}
   }
 };
 
@@ -591,6 +602,34 @@
   ConsumeAnnotationToken();
 }
 
+void Parser::HandlePragmaFEnvAccess() {
+  assert(Tok.is(tok::annot_pragma_fenv_access));
+  tok::OnOffSwitch OOS =
+static_cast(
+reinterpret_cast(Tok.getAnnotationValue()));
+
+  LangOptions::FEnvAccessModeKind FPC;
+  switch (OOS) {
+  case tok::OOS_ON:
+FPC = LangOptions::FEA_On;
+break;
+  case tok::OOS_OFF:
+FPC = LangOptions::FEA_Off;
+break;
+  case tok::OOS_DEFAULT:
+#if NOTYET // FIXME: Add this cli option when it makes sense.
+FPC = getLangOpts().getDefaultFEnvAccessMode();
+#else
+FPC = LangOptions::FEA_Off;
+#endif
+break;
+  }
+
+  Actions.ActOnPragmaFEnvAccess(FPC);
+  ConsumeAnnotationToken();
+}
+
+
 StmtResult Parser::HandlePragmaCaptured()
 {
   assert(Tok.is(tok::annot_pragma_captured));
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -8433,6 +8433,10 @@
   /// \#pragma clang fp contract
   void ActOnPragmaFPContract(LangOptions::FPContractModeKind FPC);
 
+  /// ActOnPragmaFenvAccess - Called on well formed
+  /// \#pragma STDC FENV_ACCESS
+  void ActOnPragmaFEnvAccess(LangOptions::FEnvAccessModeKind FPC);
+
   /// AddAlignmentAttributesForRecord - Adds any needed alignment attributes to
   /// a the record decl, to handle '\#pragma pack' and '\#pragma options align'.
   void AddAlignmentAttributesForRecord(RecordDecl *RD);
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -668,6 +668,10 @@
   void HandlePragmaFPContract();
 
   /// Handle the annotation token produced for
+  /// #pragma STDC FENV_ACCESS...
+  void HandlePragmaFEnvAccess();
+
+  /// \brief Handle the annotation token produced for
   /// #pragma clang fp ...
   void HandlePragm

[PATCH] D49865: Inform the AST of #pragma FENV_ACCESS use

2018-07-30 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn marked 3 inline comments as done.
kpn added inline comments.



Comment at: include/clang/Basic/LangOptions.h:273-280
   FPOptions() : fp_contract(LangOptions::FPC_Off) {}
 
   // Used for serializing.
   explicit FPOptions(unsigned I)
   : fp_contract(static_cast(I)) {}
 
   explicit FPOptions(const LangOptions &LangOpts)

rsmith wrote:
> These constructors need to be updated.
I think this is what you intend?


https://reviews.llvm.org/D49865



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


r338290 - Make test/Driver/baremetal.cpp work with linkers other than lld

2018-07-30 Thread David Greene via cfe-commits
Author: greened
Date: Mon Jul 30 12:08:20 2018
New Revision: 338290

URL: http://llvm.org/viewvc/llvm-project?rev=338290&view=rev
Log:
Make test/Driver/baremetal.cpp work with linkers other than lld

This test fails if clang is configure with, for example, gold as the
default linker. It does not appear that this test really relies on lld
so make the checks accept ld, ld.gold and ld.bfd too.


Modified:
cfe/trunk/test/Driver/baremetal.cpp

Modified: cfe/trunk/test/Driver/baremetal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/baremetal.cpp?rev=338290&r1=338289&r2=338290&view=diff
==
--- cfe/trunk/test/Driver/baremetal.cpp (original)
+++ cfe/trunk/test/Driver/baremetal.cpp Mon Jul 30 12:08:20 2018
@@ -10,7 +10,7 @@
 // CHECK-V6M-C-SAME: "-internal-isystem" 
"[[SYSROOT]]{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
-// CHECK-V6M-C-NEXT: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" 
"-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
 // CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -32,7 +32,7 @@
 // RUN: -target armv6m-none-eabi \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
-// CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-DEFAULTCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -45,7 +45,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
 // CHECK-V6M-LIBCXX-NOT: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}{{[^v].*}}"
 // CHECK-V6M-LIBCXX: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECK-V6M-LIBCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
 // CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -58,7 +58,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
 // CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-V6M-LIBSTDCXX: "-internal-isystem" 
"{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
-// CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" 
"{{.*}}.o" "-Bstatic"
 // CHECK-V6M-LIBSTDCXX-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lstdc++" "-lsupc++" "-lunwind"
 // CHECK-V6M-LIBSTDCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m.a"
@@ -69,7 +69,7 @@
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -nodefaultlibs \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-NDL %s
-// CHECK-V6M-NDL: "{{[^"]*}}ld.lld{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-V6M-NDL: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" 
"-Bstatic"
 // CHECK-V6M-NDL-SAME: 
"-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
 "-o" "{{.*}}.o"
 
 // RUN: %clangxx -target arm-none-eabi -v 2>&1 \


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


[PATCH] D41376: [libcxx] Implement ABI for Clang/GCC vector extension, constructors, copy_from and copy_to.

2018-07-30 Thread Tim Shen via Phabricator via cfe-commits
timshen updated this revision to Diff 158038.
timshen marked 2 inline comments as done.
timshen added a comment.

Update based on comments.


https://reviews.llvm.org/D41376

Files:
  libcxx/include/__config
  libcxx/include/experimental/__config
  libcxx/include/experimental/simd
  libcxx/test/std/experimental/simd/simd.abi/vector_extension.pass.cpp
  libcxx/test/std/experimental/simd/simd.access/default.pass.cpp
  libcxx/test/std/experimental/simd/simd.casts/simd_cast.pass.cpp
  libcxx/test/std/experimental/simd/simd.casts/static_simd_cast.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/broadcast.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/default.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/generator.pass.cpp
  libcxx/test/std/experimental/simd/simd.cons/load.pass.cpp
  libcxx/test/std/experimental/simd/simd.mem/load.pass.cpp
  libcxx/test/std/experimental/simd/simd.mem/store.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/abi_for_size.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_abi_tag.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd_flag_type.pass.cpp
  libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp

Index: libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp
===
--- libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp
+++ libcxx/test/std/experimental/simd/simd.traits/is_simd_mask.pass.cpp
@@ -7,127 +7,146 @@
 //
 //===--===//
 
-// UNSUPPORTED: c++98, c++03
+// UNSUPPORTED: c++98, c++03, c++11, c++14
 
 // 
 //
 // [simd.traits]
-// template  struct is_simd_mask;
-// template  inline constexpr bool is_simd_mask_v = is_simd_mask::value;
+// template  struct ex::is_simd_mask;
+// template  inline constexpr bool ex::is_simd_mask_v =
+// ex::is_simd_mask::value;
 
 #include 
 #include 
 #include "test_macros.h"
 
-using namespace std::experimental::parallelism_v2;
+namespace ex = std::experimental::parallelism_v2;
 
 struct UserType {};
 
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-static_assert( is_simd_mask>::value, "");
-
-static_assert(!is_simd_mask::value, "");
-static_assert(!is_simd_mask::value, "");
-static_assert(!is_simd_mask::value, "");
-static_assert(!is_simd_mask>::value, "");
-static_assert(!is_simd_mask>::value, "");
-static_assert(!is_simd_mask::value, "");
-
-#if TEST_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES) &&\
-!defined(_LIBCPP_HAS_NO_INLINE_VARIABLES)
-
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_assert( is_simd_mask_v>, "");
-static_

[PATCH] D41376: [libcxx] Implement ABI for Clang/GCC vector extension, constructors, copy_from and copy_to.

2018-07-30 Thread Tim Shen via Phabricator via cfe-commits
timshen added inline comments.



Comment at: libcxx/include/experimental/simd:703
+public:
+  _Tp __get(size_t __index) const { return (&__storage_)[__index]; };
+  void __set(size_t __index, _Tp __val) { (&__storage_)[__index] = __val; }

mclow.lists wrote:
> Can these (`__get` and `__set`) be noexcept? Obviously, it depends on `_Tp`.
Currently _Tp is always a subset of arithmetic type, so it's safe to say 
noexcept. Added them, as they may help with -O0 performance.



Comment at: libcxx/include/experimental/simd:811
+class __simd_reference {
+  static_assert(std::is_same<_Vp, _Tp>::value, "");
+

mclow.lists wrote:
> If `_Vp` and `_Tp` have to name the same type, why have two of them?
> What is the difference, and when would you use each one internally?
> 
> __simd_reference(__simd_storage<_Tp, _Abi>* __ptr, size_t __index);
> vs.
> __simd_reference operator=(_Vp __value) &&;
> 
> If _Vp and _Tp have to name the same type, why have two of them?

Currently there is no difference. It's going to be different when simd_mask is 
implemented (_Vp can be bool, but _Tp may not be).

> What is the difference, and when would you use each one internally?

This is not an internal class. This is part of the interface. IIRC since 
P0214R9 opreator=() is changed to something. In a later patch operator=() 
should be implemented as R9 specified.


https://reviews.llvm.org/D41376



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


[PATCH] D49898: Make test/Driver/baremetal.cpp work with linkers other than lld

2018-07-30 Thread David Greene via Phabricator via cfe-commits
greened closed this revision.
greened added a comment.

Committed in 338290.


Repository:
  rC Clang

https://reviews.llvm.org/D49898



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


Re: r338239 - [mips64][clang] Provide the signext attribute for i32 return values

2018-07-30 Thread Friedman, Eli via cfe-commits

On 7/30/2018 3:44 AM, Stefan Maksimovic via cfe-commits wrote:

Author: smaksimovic
Date: Mon Jul 30 03:44:46 2018
New Revision: 338239

URL: http://llvm.org/viewvc/llvm-project?rev=338239&view=rev
Log:
[mips64][clang] Provide the signext attribute for i32 return values

Additional info: see r338019.

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

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


I'd like to see some test coverage for this.

-El

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project

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


[PATCH] D50002: [coroutines] Fix handling of dependent co_await in StmtProfiler

2018-07-30 Thread Victor Zverovich via Phabricator via cfe-commits
vitaut created this revision.
vitaut added reviewers: rsmith, GorNishanov, EricWF.
Herald added subscribers: cfe-commits, modocache.

Fix "Invalid operator call kind" error (`llvm_unreachable`) in
`DecodeOperatorCall` when compiling the following example with `-emit-pch`:

  struct S {};
  S operator co_await(S) { return S(); }
  
  template 
  int f3(T x) {
co_await x;
  }

Add the example to PCH test cases.


Repository:
  rC Clang

https://reviews.llvm.org/D50002

Files:
  lib/AST/StmtProfile.cpp
  test/PCH/coroutines.cpp


Index: test/PCH/coroutines.cpp
===
--- test/PCH/coroutines.cpp
+++ test/PCH/coroutines.cpp
@@ -66,6 +66,14 @@
   co_return x;   // checks coreturn_stmt with expr
 }
 
+struct S {};
+S operator co_await(S) { return S(); }
+
+template 
+int f3(T x) {
+  co_await x; // checks dependent_coawait with overloaded co_await operator
+}
+
 #else
 
 // expected-no-diagnostics
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -1277,7 +1277,6 @@
   case OO_Arrow:
   case OO_Call:
   case OO_Conditional:
-  case OO_Coawait:
   case NUM_OVERLOADED_OPERATORS:
 llvm_unreachable("Invalid operator call kind");
   
@@ -1449,6 +1448,10 @@
   
   case OO_Subscript:
 return Stmt::ArraySubscriptExprClass;
+
+  case OO_Coawait:
+UnaryOp = UO_Coawait;
+return Stmt::UnaryOperatorClass;
   }
   
   llvm_unreachable("Invalid overloaded operator expression");


Index: test/PCH/coroutines.cpp
===
--- test/PCH/coroutines.cpp
+++ test/PCH/coroutines.cpp
@@ -66,6 +66,14 @@
   co_return x;   // checks coreturn_stmt with expr
 }
 
+struct S {};
+S operator co_await(S) { return S(); }
+
+template 
+int f3(T x) {
+  co_await x; // checks dependent_coawait with overloaded co_await operator
+}
+
 #else
 
 // expected-no-diagnostics
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -1277,7 +1277,6 @@
   case OO_Arrow:
   case OO_Call:
   case OO_Conditional:
-  case OO_Coawait:
   case NUM_OVERLOADED_OPERATORS:
 llvm_unreachable("Invalid operator call kind");
   
@@ -1449,6 +1448,10 @@
   
   case OO_Subscript:
 return Stmt::ArraySubscriptExprClass;
+
+  case OO_Coawait:
+UnaryOp = UO_Coawait;
+return Stmt::UnaryOperatorClass;
   }
   
   llvm_unreachable("Invalid overloaded operator expression");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50003: Sema: Fix explicit address space cast involving void pointers

2018-07-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: rjmccall.

Explicit cast of a void pointer to a pointer type in different address space is
incorrectly classified as bitcast, which causes invalid bitcast in codegen.

The patch fixes that by checking the address space of the source and destination
type and set the correct cast kind.


https://reviews.llvm.org/D50003

Files:
  lib/Sema/SemaCast.cpp
  test/CodeGenCXX/address-space-cast.cpp

Index: test/CodeGenCXX/address-space-cast.cpp
===
--- test/CodeGenCXX/address-space-cast.cpp
+++ test/CodeGenCXX/address-space-cast.cpp
@@ -3,13 +3,63 @@
 #define __private__ __attribute__((address_space(5)))
 
 void func_pchar(__private__ char *x);
+void func_pvoid(__private__ void *x);
+void func_pint(__private__ int *x);
 
-void test_cast(char *gen_ptr) {
+void test_cast(char *gen_char_ptr, void *gen_void_ptr, int *gen_int_ptr) {
   // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
   // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
-  __private__ char *priv_ptr = (__private__ char *)gen_ptr;
+  __private__ char *priv_char_ptr = (__private__ char *)gen_char_ptr;
 
   // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_char_ptr = (__private__ char *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_char_ptr = (__private__ char *)gen_int_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  __private__ void *priv_void_ptr = (__private__ void *)gen_char_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_void_ptr = (__private__ void *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: store i8 addrspace(5)* %[[cast]]
+  priv_void_ptr = (__private__ void *)gen_int_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i32 addrspace(5)*
+  // CHECK-NEXT: store i32 addrspace(5)* %[[cast]]
+  __private__ int *priv_int_ptr = (__private__ int *)gen_void_ptr;
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
+  func_pchar((__private__ char *)gen_char_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
+  func_pchar((__private__ char *)gen_void_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
   // CHECK-NEXT: call void @_Z10func_pcharPU3AS5c(i8 addrspace(5)* %[[cast]])
-  func_pchar((__private__ char *)gen_ptr);
+  func_pchar((__private__ char *)gen_int_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_char_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_void_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(5)*
+  // CHECK-NEXT: call void @_Z10func_pvoidPU3AS5v(i8 addrspace(5)* %[[cast]])
+  func_pvoid((__private__ void *)gen_int_ptr);
+
+  // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i32 addrspace(5)*
+  // CHECK-NEXT: call void @_Z9func_pintPU3AS5i(i32 addrspace(5)* %[[cast]])
+  func_pint((__private__ int *)gen_void_ptr);
 }
Index: lib/Sema/SemaCast.cpp
===
--- lib/Sema/SemaCast.cpp
+++ lib/Sema/SemaCast.cpp
@@ -1044,6 +1044,12 @@
   }
 }
 
+static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
+  return SrcType->isPointerType() && DestType->isPointerType() &&
+ SrcType->getAs()->getPointeeType().getAddressSpace() !=
+ DestType->getAs()->getPointeeType().getAddressSpace();
+}
+
 /// TryStaticCast - Check if a static cast can be performed, and do so if
 /// possible. If @p CStyle, ignore access restrictions on hierarchy casting
 /// and casting away constness.
@@ -1185,7 +1191,9 @@
   return TC_Failed;
 }
   }
-  Kind = CK_BitCast;
+  Kind = IsAddressSpaceConversion(SrcType, DestType)
+ ? CK_AddressSpaceConversion
+ : CK_BitCast;
   return TC_Success;
 }
 
@@ -1964,12 +1972,6 @@
   return Result.isUsable();
 }
 
-static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
-  return SrcType->isPointerType() && DestType->isPointerType() &&
- SrcType->getAs()->getPointeeType().getA

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

2018-07-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D49930#1180388, @scott.linder wrote:

> Thank you for taking a look @yaxunl. Should I wait for another reviewer or 
> can I commit this?


I think it is OK to land. We could have post-commit reviews if there are any 
issues.


Repository:
  rC Clang

https://reviews.llvm.org/D49930



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


[PATCH] D48862: [OpenEmbedded] Fix lib paths for OpenEmbedded targets

2018-07-30 Thread Mandeep Singh Grang via Phabricator via cfe-commits
mgrang added a comment.

I would commit this today unless there any comments. Thanks.


https://reviews.llvm.org/D48862



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


[PATCH] D50002: [coroutines] Fix handling of dependent co_await in StmtProfiler

2018-07-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Looks good, thanks. Do you need someone to commit this for you?


Repository:
  rC Clang

https://reviews.llvm.org/D50002



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


[PATCH] D50002: [coroutines] Fix handling of dependent co_await in StmtProfiler

2018-07-30 Thread Victor Zverovich via Phabricator via cfe-commits
vitaut added a comment.

> Do you need someone to commit this for you?

Yes, please. I don't have commit rights.


Repository:
  rC Clang

https://reviews.llvm.org/D50002



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


r338294 - [OpenEmbedded] Fix lib paths for OpenEmbedded targets

2018-07-30 Thread Mandeep Singh Grang via cfe-commits
Author: mgrang
Date: Mon Jul 30 12:44:13 2018
New Revision: 338294

URL: http://llvm.org/viewvc/llvm-project?rev=338294&view=rev
Log:
[OpenEmbedded] Fix lib paths for OpenEmbedded targets

Summary:
The lib paths are not correctly picked up for OpenEmbedded sysroots (like 
arm-oe-linux-gnueabi) for 2 reasons:

1. OpenEmbedded sysroots are of the form /usr/lib//x.y.z. This 
form is handled in clang but only for Freescale vendor.

2. 64-bit OpenEmbedded sysroots may not have a /usr/lib dir. So they cannot 
find /usr/lib64 as it is referenced as /usr/lib/../lib64 in clang.

This is a follow-up to the llvm patch: D48861

Reviewers: dlj, rengolin, fedor.sergeev, javed.absar, hfinkel, rsmith

Reviewed By: rsmith

Subscribers: rsmith, kristof.beyls, cfe-commits

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

Added:
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/c++/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/c++/6.3.0/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/c++/6.3.0/backward/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/c++/6.3.0/backward/.keep
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/6.3.0/

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/6.3.0/crtbegin.o

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/6.3.0/crtend.o

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crt1.o

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crti.o

cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crtn.o
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/c++/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/c++/6.3.0/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/c++/6.3.0/backward/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/c++/6.3.0/backward/.keep
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/6.3.0/

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/6.3.0/crtbegin.o

cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/6.3.0/crtend.o
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crt1.o
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crti.o
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crtn.o
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/test/Driver/linux-header-search.cpp
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=338294&r1=338293&r2=338294&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Mon Jul 30 12:44:13 2018
@@ -2188,7 +2188,8 @@ void Generic_GCC::GCCInstallationDetecto
   // this on Freescale triples, though, since some systems put a *lot* of
   // files in that location, not just GCC installation data.
   {CandidateTriple.str(), "..",
-   TargetTriple.getVendor() == llvm::Triple::Freescale},
+   TargetTriple.getVendor() == llvm::Triple::Freescale ||
+   TargetTriple.getVendor() == llvm::Triple::OpenEmbedded},
 
   // Natively multiarch systems sometimes put the GCC triple-specific
   // directory within their multiarch lib directory, resulting in the

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=338294&r1=338293&r2=338294&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Mon Jul 30 12:44:13 2018
@@ -376,7 +376,14 @@ Linux::Linux(const Driver &D, 

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

After thinking about this for some more, I'm not sure this patch is worth doing 
in its current form. The minimal patch for allowing specializations of 
`allocator_traits` would be:

1. move the `__move_construct_forward` & friends functions from 
`allocator_traits` to private static member functions of `std::vector` (because 
they're only used in `std::vector` right now).
2. keep the SFINAE on the allocator and avoid encoding any `memcpy` decision at 
the call site.

However, an even better alternative would be to look into adding an overload to 
`uninitialized_move` & friends that takes an allocator. We could then be clever 
in how this is implemented. The major benefit I see here is that there would be 
one common code path to optimize, as opposed to a `std::vector`-specific code 
path.

Given the small benefit provided by this patch, my opinion is that it's not 
worth moving forward with it as-is. However, I believe either of the two 
alternatives suggested above would be welcome, with a preference for the more 
comprehensive second approach, which requires a paper.

Arthur, what do you think? Do you think the second approach can work?


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


[PATCH] D48862: [OpenEmbedded] Fix lib paths for OpenEmbedded targets

2018-07-30 Thread Mandeep Singh Grang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338294: [OpenEmbedded] Fix lib paths for OpenEmbedded 
targets (authored by mgrang, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48862?vs=157391&id=158049#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48862

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/lib/Driver/ToolChains/Linux.cpp
  
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/include/c++/6.3.0/backward/.keep
  
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/6.3.0/crtbegin.o
  
cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/aarch64-oe-linux/6.3.0/crtend.o
  cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crt1.o
  cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crti.o
  cfe/trunk/test/Driver/Inputs/openembedded_aarch64_linux_tree/usr/lib64/crtn.o
  
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/include/c++/6.3.0/backward/.keep
  
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/6.3.0/crtbegin.o
  
cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/arm-oe-linux-gnueabi/6.3.0/crtend.o
  cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crt1.o
  cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crti.o
  cfe/trunk/test/Driver/Inputs/openembedded_arm_linux_tree/usr/lib/crtn.o
  cfe/trunk/test/Driver/linux-header-search.cpp
  cfe/trunk/test/Driver/linux-ld.c

Index: cfe/trunk/test/Driver/linux-ld.c
===
--- cfe/trunk/test/Driver/linux-ld.c
+++ cfe/trunk/test/Driver/linux-ld.c
@@ -1813,4 +1813,40 @@
 // CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
 // CHECK-LD-AMI: "-lc"
 // CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
-//
+
+// Check whether the OpenEmbedded ARM libs are added correctly.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=arm-oe-linux-gnueabi \
+// RUN: --sysroot=%S/Inputs/openembedded_arm_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OE-ARM %s
+
+// CHECK-OE-ARM: "-cc1" "-triple" "armv4t-oe-linux-gnueabi"
+// CHECK-OE-ARM: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-OE-ARM: "-m" "armelf_linux_eabi" "-dynamic-linker"
+// CHECK-OE-ARM: "[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0/../../../lib{{/|}}crt1.o"
+// CHECK-OE-ARM: "[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0/../../../lib{{/|}}crti.o"
+// CHECK-OE-ARM: "[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0{{/|}}crtbegin.o"
+// CHECK-OE-ARM: "-L[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0"
+// CHECK-OE-ARM: "-L[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi"
+// CHECK-OE-ARM: "-L[[SYSROOT]]/usr/lib"
+// CHECK-OE-ARM: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-OE-ARM: "[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0{{/|}}crtend.o"
+// CHECK-OE-ARM: "[[SYSROOT]]/usr/lib/arm-oe-linux-gnueabi/6.3.0/../../../lib{{/|}}crtn.o"
+
+// Check whether the OpenEmbedded AArch64 libs are added correctly.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-oe-linux \
+// RUN: --sysroot=%S/Inputs/openembedded_aarch64_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-OE-AARCH64 %s
+
+// CHECK-OE-AARCH64: "-cc1" "-triple" "aarch64-oe-linux"
+// CHECK-OE-AARCH64: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-OE-AARCH64: "-m" "aarch64linux" "-dynamic-linker"
+// CHECK-OE-AARCH64: "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../lib64{{/|}}crt1.o"
+// CHECK-OE-AARCH64: "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../lib64{{/|}}crti.o"
+// CHECK-OE-AARCH64: "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0{{/|}}crtbegin.o"
+// CHECK-OE-AARCH64: "-L[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0"
+// CHECK-OE-AARCH64: "-L[[SYSROOT]]/usr/lib64"
+// CHECK-OE-AARCH64: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-OE-AARCH64: "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0{{/|}}crtend.o"
+// CHECK-OE-AARCH64: "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../lib64{{/|}}crtn.o"
Index: cfe/trunk/test/Driver/linux-header-search.cpp
===
--- cfe/trunk/test/Driver/linux-header-search.cpp
+++ cfe/trunk/test/Driver/linux-header-search.cpp
@@ -493,3 +493,25 @@
 // CHECK-DEBIAN-SPARC64: "-internal-externc-isystem" "[[SYSROOT]]/usr/include/sparc64-linux-gnu"
 // CHECK-DEBIAN-SPARC64: "-internal-externc-isystem" "[[SYSROOT]]/include"
 // CHECK-DEBIAN-SPARC64: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+
+// Check header search on OpenEmbedded ARM.
+// RUN: %clang -no-ca

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2018-07-30 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

I think we just ran into one such issue. We're using our own Clang that's 
usually following tip-of-tree and we recently switched to C++17, but that 
started failing on our macOS 10.12 bots with:

  Undefined symbols for architecture x86_64:
"operator delete(void*, std::align_val_t)", referenced from:
_main in main.cpp.o
std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() in main.cpp.o

std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() 
in main.cpp.o
std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() in 
main.cpp.o
std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() in 
main.cpp.o

std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() 
in main.cpp.o
std::__1::__vector_base >, 
std::__1::allocator > > >::~__vector_base() in 
main.cpp.o
...
"operator new(unsigned long, std::align_val_t)", referenced from:
std::__1::enable_if<__is_forward_iterator::value, void>::type 
std::__1::basic_string, 
std::__1::allocator >::__init(char*, char*) in main.cpp.o

std::__1::unique_ptr 
> >, void*>, 
std::__1::__tree_node_destructor > >, void*> > > > 
std::__1::__tree > >, 
std::__1::__map_value_compare<(anonymous namespace)::Behavior, 
std::__1::__value_type<(anonymous namespace)::Behavior, 
std::__1::basic_fstream > >, 
std::__1::less<(anonymous namespace)::Behavior>, true>, 
std::__1::allocator > > > 
>::__construct_node<(anonymous namespace)::Behavior, 
std::__1::basic_fstream > >((anonymous 
namespace)::Behavior&&, std::__1::basic_fstream >&&) in main.cpp.o
std::__1::__split_buffer&>::__split_buffer(unsigned long, 
unsigned long, std::__1::allocator&) in main.cpp.o
std::__1::vector >::__vallocate(unsigned long) in 
main.cpp.o

std::__1::unique_ptr >, 
std::__1::unique_ptr > >, void*>, 
std::__1::__tree_node_destructor >, 
std::__1::unique_ptr > >, void*> > > > 
std::__1::__tree >, 
std::__1::unique_ptr > >, 
std::__1::__map_value_compare >, 
std::__1::__value_type >, 
std::__1::unique_ptr > >, 
std::__1::less > >, true>, 
std::__1::allocator >, 
std::__1::unique_ptr > > > 
>::__construct_node >, 
std::__1::unique_ptr > > 
>(std::__1::pair >, 
std::__1::unique_ptr > >&&) in main.cpp.o
std::__1::enable_if<__is_forward_iterator::value, 
void>::type std::__1::basic_string, 
std::__1::allocator >::__init(char const*, char const*) in 
libfidl.a(c_generator.cpp.o)
std::__1::__split_buffer&>::__split_buffer(unsigned long, unsigned long, 
std::__1::allocator&) in libfidl.a(c_generator.cpp.o)
...
  ld: symbol(s) not found for architecture x86_64

AFAICT this is because `/usr/lib/libc++.dylib` doesn't yet support C++17, but 
the headers that are part of Clang toolchain do. When we force Clang to use 
libc++ that's part of the toolchain by manually setting the necessary `-L`/`-l` 
flag resolves the issue. I don't know if this is an expected behavior, but I'd 
really appreciate some response on this.


Repository:
  rC Clang

https://reviews.llvm.org/D45639



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


[PATCH] D41376: [libcxx] Implement ABI for Clang/GCC vector extension, constructors, copy_from and copy_to.

2018-07-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


https://reviews.llvm.org/D41376



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


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

2018-07-30 Thread Martin Böhme via Phabricator via cfe-commits
mboehme updated this revision to Diff 158051.
mboehme added a comment.

Various changes in response to reviewer comments.


Repository:
  rC Clang

https://reviews.llvm.org/D49911

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


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


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

Re: r338291 - Remove trailing space

2018-07-30 Thread Fāng-ruì Sòng via cfe-commits
Oops.. sorry but now they have been committed..
On Mon, Jul 30, 2018 at 12:31 PM Aaron Ballman  wrote:
>
> This is an extraordinary amount of churn for very little value, IMO.
> The same goes for r338291. Were these changes discussed somewhere
> before being committed? I worry about the negative impact for third
> parties having to deal with changes on this scale.
>
> ~Aaron
>
> On Mon, Jul 30, 2018 at 3:24 PM, Fangrui Song via cfe-commits
>  wrote:
> > Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=338291&r1=338290&r2=338291&view=diff
> > ==
> > --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Mon Jul 30 12:24:48 2018
> > @@ -73,7 +73,7 @@ ExprResult Sema::ParseObjCStringLiteral(
> >/*Pascal=*/false, StrTy, &StrLocs[0],
> >StrLocs.size());
> >}
> > -
> > +
> >return BuildObjCStringLiteral(AtLocs[0], S);
> >  }
> >
> > @@ -92,12 +92,12 @@ ExprResult Sema::BuildObjCStringLiteral(
> >} else if (getLangOpts().NoConstantCFStrings) {
> >  IdentifierInfo *NSIdent=nullptr;
> >  std::string StringClass(getLangOpts().ObjCConstantStringClass);
> > -
> > +
> >  if (StringClass.empty())
> >NSIdent = &Context.Idents.get("NSConstantString");
> >  else
> >NSIdent = &Context.Idents.get(StringClass);
> > -
> > +
> >  NamedDecl *IF = LookupSingleName(TUScope, NSIdent, AtLoc,
> >   LookupOrdinaryName);
> >  if (ObjCInterfaceDecl *StrIF = 
> > dyn_cast_or_null(IF)) {
> > @@ -126,10 +126,10 @@ ExprResult Sema::BuildObjCStringLiteral(
> >// being an 'id' type.
> >Ty = Context.getObjCNSStringType();
> >if (Ty.isNull()) {
> > -ObjCInterfaceDecl *NSStringIDecl =
> > -  ObjCInterfaceDecl::Create (Context,
> > - Context.getTranslationUnitDecl(),
> > - SourceLocation(), NSIdent,
> > +ObjCInterfaceDecl *NSStringIDecl =
> > +  ObjCInterfaceDecl::Create (Context,
> > + Context.getTranslationUnitDecl(),
> > + SourceLocation(), NSIdent,
> >   nullptr, nullptr, SourceLocation());
> >  Ty = Context.getObjCInterfaceType(NSStringIDecl);
> >  Context.setObjCNSStringType(Ty);
> > @@ -252,16 +252,16 @@ static ObjCMethodDecl *getNSNumberFactor
> >  }
> >  return nullptr;
> >}
> > -
> > +
> >// If we already looked up this method, we're done.
> >if (S.NSNumberLiteralMethods[*Kind])
> >  return S.NSNumberLiteralMethods[*Kind];
> > -
> > +
> >Selector Sel = S.NSAPIObj->getNSNumberLiteralSelector(*Kind,
> >  
> > /*Instance=*/false);
> > -
> > +
> >ASTContext &CX = S.Context;
> > -
> > +
> >// Look up the NSNumber class, if we haven't done so already. It's cached
> >// in the Sema instance.
> >if (!S.NSNumberDecl) {
> > @@ -277,7 +277,7 @@ static ObjCMethodDecl *getNSNumberFactor
> >  QualType NSNumberObject = CX.getObjCInterfaceType(S.NSNumberDecl);
> >  S.NSNumberPointer = CX.getObjCObjectPointerType(NSNumberObject);
> >}
> > -
> > +
> >// Look for the appropriate method within NSNumber.
> >ObjCMethodDecl *Method = S.NSNumberDecl->lookupClassMethod(Sel);
> >if (!Method && S.getLangOpts().DebuggerObjCLiteral) {
> > @@ -304,7 +304,7 @@ static ObjCMethodDecl *getNSNumberFactor
> >
> >// Note: if the parameter type is out-of-line, we'll catch it later in 
> > the
> >// implicit conversion.
> > -
> > +
> >S.NSNumberLiteralMethods[*Kind] = Method;
> >return Method;
> >  }
> > @@ -322,21 +322,21 @@ ExprResult Sema::BuildObjCNumericLiteral
> >  case CharacterLiteral::UTF8:
> >NumberType = Context.CharTy;
> >break;
> > -
> > +
> >  case CharacterLiteral::Wide:
> >NumberType = Context.getWideCharType();
> >break;
> > -
> > +
> >  case CharacterLiteral::UTF16:
> >NumberType = Context.Char16Ty;
> >break;
> > -
> > +
> >  case CharacterLiteral::UTF32:
> >NumberType = Context.Char32Ty;
> >break;
> >  }
> >}
> > -
> > +
> >// Look for the appropriate method within NSNumber.
> >// Construct the literal.
> >SourceRange NR(Number->getSourceRange());
> > @@ -355,33 +355,33 @@ ExprResult Sema::BuildObjCNumericLiteral
> >if (ConvertedNumber.isInvalid())
> >  return ExprError();
> >Number = ConvertedNumber.get();
> > -
> > +
> >// Use the effective source range of the literal, including the leading 
> > '@'.
> >return MaybeBindToTemporary(
> > new (Context) ObjCBoxedExpr(Number, NSNumberPointer, Meth

[PATCH] D49865: Inform the AST of #pragma FENV_ACCESS use

2018-07-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Parse/ParsePragma.cpp:619-623
+#if NOTYET // FIXME: Add this cli option when it makes sense.
+  case tok::OOS_DEFAULT:
+FPC = getLangOpts().getDefaultFENVAccessMode();
+break;
+#endif

rsmith wrote:
> You need to do *something* in this case. Currently, `FPC` is read 
> uninitialized a few lines below when this happens. How about just treating 
> this as the same as `OFF` for now, since that is our default.
Drop the `#if NOTYET` part; we don't like having checked-in but commented-out 
code. Just keep the FIXME comment.



Comment at: lib/Parse/ParsePragma.cpp:109
  return;
-if (OOS == tok::OOS_ON)
+if (OOS == tok::OOS_ON) {
   PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);

We also need to pass the pragma on to `Sema` when it's set to `OFF`.


https://reviews.llvm.org/D49865



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


[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

2018-07-30 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

In https://reviews.llvm.org/D49317#1180852, @ldionne wrote:

> After thinking about this for some more, I'm not sure this patch is worth 
> doing in its current form. The minimal patch for allowing specializations of 
> `allocator_traits` would be:
>
> 1. move the `__move_construct_forward` & friends functions from 
> `allocator_traits` to private static member functions of `std::vector` 
> (because they're only used in `std::vector` right now).
> 2. keep the SFINAE on the allocator and avoid encoding any `memcpy` decision 
> at the call site.


FWLIW, I approve of (1) but not (2), for the previously stated reason that the 
optimal path is known only at the call-site; the callee doesn't have enough 
information to know whether memcpy is appropriate.  (But it sounds like 
Marshall doesn't want any memcpy happening at all, so maybe it's moot?)

> However, an even better alternative would be to look into adding an overload 
> to `uninitialized_move` & friends that takes an allocator. We could then be 
> clever in how this is implemented. The major benefit I see here is that there 
> would be one common code path to optimize, as opposed to a 
> `std::vector`-specific code path.

Yes, when I implemented https://github.com/Quuxplusone/from-scratch/, one of 
the many things I noticed was that none of the uninitialized_foo algorithms 
were useful out of the box; every one of them needed to be reimplemented to 
take an allocator parameter. (A.k.a., "scoped_allocator_adaptor is why we can't 
have nice things.")  However, as you point out, this is a long-standing problem 
and would require a library paper to do "right." (It would still be easy enough 
to add the needed algorithms with uglified names, e.g. 
`__uninitialized_copy_a`, `__destroy_a`, etc. This is exactly what libstdc++ 
does, and libc++ might be wise to copy its approach.)

I'd be happy to throw together a patch for `__uninitialized_copy_a` etc., since 
I think that would improve libc++ in general; but I don't see how that would 
directly help any specific short-term problem in libc++. This patch as it is 
helps two specific short-term problems:
(1) that user specializations of allocator_traits don't work (but, as the test 
case comments, this is arguably not a good idea anyway; see also 
https://quuxplusone.github.io/blog/2018/07/14/traits-classes/ )
(2) that the diff between libc++ trunk and libc++ trivially-relocatable is 
unnecessarily large
Messing with the uninitialized_foo algorithms would not directly help either of 
these problems, so we'd have to come up with some other rationale for it.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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


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

2018-07-30 Thread Martin Böhme via Phabricator via cfe-commits
mboehme marked 3 inline comments as done.
mboehme added a comment.

> Should this attribute have some semantic checking that ensures the non-static 
> data members are accessed in the function that claims it reinitializes the 
> object?

I think this would be hard to do in a way that provides meaningful value.

We can't demand that the function should modify all member variables because 
not all functions that perform a logical "clear" or other reinitialization need 
to do this. For example, a typical implementation of `std::string::clear()` 
only modifies the string length but leaves the capacity and buffer unchanged.

The strongest requirement I think we can impose is that the function needs to 
modify at least one of the member variables -- possibly indirectly (i.e. 
through a call to another function). I don't think checking this would provide 
a real benefit though. We already disallow the `reinitializes` attribute for 
const member functions -- so if someone misuses the attribute, it'll be on a 
non-const member function that most likely modifies member variables, just not 
in a way that's guaranteed to reinitialize the object.

What we'd really want to check is whether the function is actually doing a 
logical reinitialization of the object. Instead of putting the 'reinitializes' 
attribute on the function, we'd want some way of expressing "this function has 
the precondition `true` and the postcondition `empty()`", and then we'd like to 
prove this at compile time or at least check it at runtime. That's obviously 
beyond the scope of what is possible in C++ today though.




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

aaron.ballman wrote:
> `non-static, non-const member functions` (with the comma)
IIUC, Clang then translates the comma into an "and" -- so the actual diagnostic 
becomes "non-static and non-const member functions" (see the expected-error in 
the tests). Is this as intended?



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

aaron.ballman wrote:
> I think it makes sense to use `CXX11` instead of `Clang` as this attribute 
> doesn't make sense in C2x mode, but should there be a `GNU` spelling as well?
I have to admit I'm not sure. What are the usual considerations here? Does this 
need to be coordinated in any way with GNU, or can Clang simply introduce 
additional "GNU-style" spellings (i.e. with `__attribute__`) that are 
Clang-only?

I understand there's a difference between `GCC` spellings and `GNU` spellings 
-- but I'm not sure what the rules around the latter are. Let me know if you 
think this should have a `GNU` spelling, and I'll add it!



Comment at: include/clang/Basic/AttrDocs.td:3426
+  let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire

aaron.ballman wrote:
> While I kind of understand the restriction on a `const` member function, what 
> about code that has mutable members being reset within a const method?
I find it hard to envision a method that reinitializes an object by modifying 
only mutable member variables. Mutable members are, after all, intended to be 
used for purposes (such as caching and locking) that do not change the "logical 
state" of the object, whereas reinitialization is an operation that does change 
the logical state of the object.

If it really does turn out that there are legitimate use cases for applying the 
'reinitializes' attribute to a const member function, we can always relax this 
requirement later on.


Repository:
  rC Clang

https://reviews.llvm.org/D49911



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


[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

2018-07-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

The patch is fine, in general, couple of comments:

a) Can you refactor this so the if conditionals are just two functions? Those 
functions are big enough already.
b) I'm not quite sure why you're picking limited here, do you have an 
explanation?
c) Can you split that part out into a separate test? Additional run lines in 
the existing blocks.cl test would be fine.

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D49930



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


  1   2   >