[PATCH] D30874: clang-format: [JS] do not wrap after interface and type.

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D30874



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

[PATCH] D30881: Track skipped files in dependency scanning

[Pete Cooper via Phabricator via cfe-commits]

pete created this revision.

Its possible for a header to be a symlink to another header.  In this case, 
both are actually the same underlying file, and will both have the same include 
guards.

Because of this, if we #include the header, then #include the symlink, or vice 
versa, then one will get a FileChanged callback, and the next FileSkipped.

So that we get an accurate dependency file, we therefore need to also implement 
the FileSkipped callback in dependency scanning.


https://reviews.llvm.org/D30881

Files:
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen-symlink.c


Index: test/Frontend/dependency-gen-symlink.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-symlink.c
@@ -0,0 +1,20 @@
+// REQUIRES: shell
+
+// Basic test
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+// RUN: mkdir %t.dir/a
+// RUN: mkdir %t.dir/b
+// RUN: echo "#ifndef HEADER_A" > %t.dir/a/header.h
+// RUN: echo "#define HEADER_A" >> %t.dir/a/header.h
+// RUN: echo "#endif" >> %t.dir/a/header.h
+// RUN: ln %t.dir/a/header.h %t.dir/b/header.h
+
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-symlink.o
+// CHECK: dependency-gen-symlink.c
+// CHECK: a/header.h
+// CHECK: b/header.h
+#include "a/header.h"
+#include "b/header.h"
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -185,6 +185,11 @@
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
FileID PrevFID) override;
+
+  void FileSkipped(const FileEntry &SkippedFile,
+   const Token &FilenameTok,
+   SrcMgr::CharacteristicKind FileType) override;
+
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
@@ -291,6 +296,12 @@
   AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
 }
 
+void DFGImpl::FileSkipped(const FileEntry &SkippedFile,
+ const Token &FilenameTok,
+ SrcMgr::CharacteristicKind FileType) {
+  AddFilename(llvm::sys::path::remove_leading_dotslash(SkippedFile.getName()));
+}
+
 void DFGImpl::InclusionDirective(SourceLocation HashLoc,
  const Token &IncludeTok,
  StringRef FileName,


Index: test/Frontend/dependency-gen-symlink.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-symlink.c
@@ -0,0 +1,20 @@
+// REQUIRES: shell
+
+// Basic test
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+// RUN: mkdir %t.dir/a
+// RUN: mkdir %t.dir/b
+// RUN: echo "#ifndef HEADER_A" > %t.dir/a/header.h
+// RUN: echo "#define HEADER_A" >> %t.dir/a/header.h
+// RUN: echo "#endif" >> %t.dir/a/header.h
+// RUN: ln %t.dir/a/header.h %t.dir/b/header.h
+
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-symlink.o
+// CHECK: dependency-gen-symlink.c
+// CHECK: a/header.h
+// CHECK: b/header.h
+#include "a/header.h"
+#include "b/header.h"
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -185,6 +185,11 @@
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
FileID PrevFID) override;
+
+  void FileSkipped(const FileEntry &SkippedFile,
+   const Token &FilenameTok,
+   SrcMgr::CharacteristicKind FileType) override;
+
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
@@ -291,6 +296,12 @@
   AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
 }
 
+void DFGImpl::FileSkipped(const FileEntry &SkippedFile,
+ const Token &FilenameTok,
+ SrcMgr::CharacteristicKind FileType) {
+  AddFilename(llvm::sys::path::remove_leading_dotslash(SkippedFile.getName()));
+}
+
 void DFGImpl::InclusionDirective(SourceLocation HashLoc,
  const Token &IncludeTok,
  StringRef FileName,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r297605 - clang-format: [JS] do not wrap after interface and type.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Mon Mar 13 02:10:18 2017
New Revision: 297605

URL: http://llvm.org/viewvc/llvm-project?rev=297605&view=rev
Log:
clang-format: [JS] do not wrap after interface and type.

Summary:
`interface` and `type` are pseudo keywords and cause automatic semicolon
insertion when followed by a line break:

interface  // gets parsed as a long variable access to "interface"
VeryLongInterfaceName {

}

With this change, clang-format not longer wraps after `interface` or `type`.

Reviewers: djasper

Subscribers: cfe-commits, klimek

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

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=297605&r1=297604&r2=297605&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Mar 13 02:10:18 2017
@@ -2526,11 +2526,10 @@ bool TokenAnnotator::canBreakBefore(cons
   return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 const FormatToken *NonComment = Right.getPreviousNonComment();
-if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw) ||
-(NonComment &&
- NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw)))
+if (NonComment &&
+NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+tok::kw_throw, Keywords.kw_interface,
+Keywords.kw_type))
   return false; // Otherwise a semicolon is inserted.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=297605&r1=297604&r2=297605&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Mar 13 02:10:18 2017
@@ -1228,6 +1228,20 @@ TEST_F(FormatTestJS, TypeAliases) {
"class C {}");
 }
 
+TEST_F(FormatTestJS, TypeInterfaceLineWrapping) {
+  const FormatStyle &Style = getGoogleJSStyleWithColumns(20);
+  verifyFormat("type LongTypeIsReallyUnreasonablyLong =\n"
+   "string;\n",
+   "type LongTypeIsReallyUnreasonablyLong = string;\n",
+   Style);
+  verifyFormat(
+  "interface AbstractStrategyFactoryProvider {\n"
+  "  a: number\n"
+  "}\n",
+  "interface AbstractStrategyFactoryProvider { a: number }\n",
+  Style);
+}
+
 TEST_F(FormatTestJS, Modules) {
   verifyFormat("import SomeThing from 'some/module.js';");
   verifyFormat("import {X, Y} from 'some/module.js';");


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

[PATCH] D30874: clang-format: [JS] do not wrap after interface and type.

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297605: clang-format: [JS] do not wrap after interface and 
type. (authored by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D30874?vs=91500&id=91519#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30874

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2526,11 +2526,10 @@
   return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 const FormatToken *NonComment = Right.getPreviousNonComment();
-if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw) ||
-(NonComment &&
- NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw)))
+if (NonComment &&
+NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+tok::kw_throw, Keywords.kw_interface,
+Keywords.kw_type))
   return false; // Otherwise a semicolon is inserted.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1228,6 +1228,20 @@
"class C {}");
 }
 
+TEST_F(FormatTestJS, TypeInterfaceLineWrapping) {
+  const FormatStyle &Style = getGoogleJSStyleWithColumns(20);
+  verifyFormat("type LongTypeIsReallyUnreasonablyLong =\n"
+   "string;\n",
+   "type LongTypeIsReallyUnreasonablyLong = string;\n",
+   Style);
+  verifyFormat(
+  "interface AbstractStrategyFactoryProvider {\n"
+  "  a: number\n"
+  "}\n",
+  "interface AbstractStrategyFactoryProvider { a: number }\n",
+  Style);
+}
+
 TEST_F(FormatTestJS, Modules) {
   verifyFormat("import SomeThing from 'some/module.js';");
   verifyFormat("import {X, Y} from 'some/module.js';");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2526,11 +2526,10 @@
   return true;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 const FormatToken *NonComment = Right.getPreviousNonComment();
-if (Left.isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw) ||
-(NonComment &&
- NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
- tok::kw_throw)))
+if (NonComment &&
+NonComment->isOneOf(tok::kw_return, tok::kw_continue, tok::kw_break,
+tok::kw_throw, Keywords.kw_interface,
+Keywords.kw_type))
   return false; // Otherwise a semicolon is inserted.
 if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
   return false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1228,6 +1228,20 @@
"class C {}");
 }
 
+TEST_F(FormatTestJS, TypeInterfaceLineWrapping) {
+  const FormatStyle &Style = getGoogleJSStyleWithColumns(20);
+  verifyFormat("type LongTypeIsReallyUnreasonablyLong =\n"
+   "string;\n",
+   "type LongTypeIsReallyUnreasonablyLong = string;\n",
+   Style);
+  verifyFormat(
+  "interface AbstractStrategyFactoryProvider {\n"
+  "  a: number\n"
+  "}\n",
+  "interface AbstractStrategyFactoryProvider { a: number }\n",
+  Style);
+}
+
 TEST_F(FormatTestJS, Modules) {
   verifyFormat("import SomeThing from 'some/module.js';");
   verifyFormat("import {X, Y} from 'some/module.js';");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30810: Preserve vec3 type.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Could you elaborate on the motivation for this change?

I was wondering why clang (CodeGen) needed the help of a command line option to 
decide whether vec3 should be converted to vec4. Can it just preserve vec3 when 
the architecture is spir?


https://reviews.llvm.org/D30810



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

[PATCH] D30875: [X86] Add checking of the scale argument to scatter/gather builtins

[Igor Breger via Phabricator via cfe-commits]

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

LGTM,
Could you please add test  that check scale and hint parameter errors for 
BI__builtin_ia32_gatherpfdpd for example.


https://reviews.llvm.org/D30875



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

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

[Pete Cooper via Phabricator via cfe-commits]

pete created this revision.
Herald added a subscriber: nemanjai.

This adds a PP callback for the __has_include and __has_include_next directives.

Checking for the presence of a header should add it to the list of header 
dependencies so this overrides the callback in the dependency scanner.

I kept the callback intentionally simple for now.  If we want to extend it to 
track things like missing headers, just as PP->FileNotFound() does today, then 
that wouldn't be hard to add later.


https://reviews.llvm.org/D30882

Files:
  include/clang/Lex/PPCallbacks.h
  lib/Frontend/DependencyFile.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/Frontend/dependency-gen-has-include.c


Index: test/Frontend/dependency-gen-has-include.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-has-include.c
@@ -0,0 +1,17 @@
+// REQUIRES: shell
+
+// Basic test
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+// RUN: mkdir %t.dir/a
+// RUN: echo "#ifndef HEADER_A" > %t.dir/a/header.h
+// RUN: echo "#define HEADER_A" >> %t.dir/a/header.h
+// RUN: echo "#endif" >> %t.dir/a/header.h
+
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-has-include.o
+// CHECK: dependency-gen-has-include.c
+// CHECK: a/header.h
+#if __has_include("a/header.h")
+#endif
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1424,6 +1424,9 @@
   PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, 
LookupFromFile,
 CurDir, nullptr, nullptr, nullptr);
 
+  if (PPCallbacks *Callbacks = PP.getPPCallbacks())
+Callbacks->HasInclude(FilenameLoc, File);
+
   // Get the result value.  A result of true means the file exists.
   return File != nullptr;
 }
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -196,6 +196,8 @@
   StringRef SearchPath, StringRef RelativePath,
   const Module *Imported) override;
 
+  void HasInclude(SourceLocation Loc, const FileEntry *File) override;
+
   void EndOfMainFile() override {
 OutputDependencyFile();
   }
@@ -319,6 +321,12 @@
   }
 }
 
+void DFGImpl::HasInclude(SourceLocation Loc, const FileEntry *File) {
+  if (!File)
+return;
+  AddFilename(llvm::sys::path::remove_leading_dotslash(File->getName()));
+}
+
 void DFGImpl::AddFilename(StringRef Filename) {
   if (FilesSet.insert(Filename).second)
 Files.push_back(Filename);
Index: include/clang/Lex/PPCallbacks.h
===
--- include/clang/Lex/PPCallbacks.h
+++ include/clang/Lex/PPCallbacks.h
@@ -258,6 +258,10 @@
   virtual void Defined(const Token &MacroNameTok, const MacroDefinition &MD,
SourceRange Range) {
   }
+
+  /// \brief Callback invoked when a has_include directive is read.
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }
   
   /// \brief Hook called when a source range is skipped.
   /// \param Range The SourceRange that was skipped. The range begins at the
@@ -411,6 +415,11 @@
 Second->PragmaDiagnostic(Loc, Namespace, mapping, Str);
   }
 
+  void HasInclude(SourceLocation Loc, const FileEntry *File) override {
+First->HasInclude(Loc, File);
+Second->HasInclude(Loc, File);
+  }
+
   void PragmaOpenCLExtension(SourceLocation NameLoc, const IdentifierInfo 
*Name,
  SourceLocation StateLoc, unsigned State) override 
{
 First->PragmaOpenCLExtension(NameLoc, Name, StateLoc, State);


Index: test/Frontend/dependency-gen-has-include.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-has-include.c
@@ -0,0 +1,17 @@
+// REQUIRES: shell
+
+// Basic test
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+// RUN: mkdir %t.dir/a
+// RUN: echo "#ifndef HEADER_A" > %t.dir/a/header.h
+// RUN: echo "#define HEADER_A" >> %t.dir/a/header.h
+// RUN: echo "#endif" >> %t.dir/a/header.h
+
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-has-include.o
+// CHECK: dependency-gen-has-include.c
+// CHECK: a/header.h
+#if __has_include("a/header.h")
+#endif
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1424,6 +1424,9 @@
   PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, LookupFromFile,
 CurDir, nullptr, nullptr, nullptr);
 
+  if (PPCallbacks *Callbacks = PP.getPPCallbacks())
+Callbacks->HasInclude(FilenameLoc, File);
+
   // Get the res

[PATCH] D30810: Preserve vec3 type.

[JinGu Kang via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D30810#699006, @ahatanak wrote:

> Could you elaborate on the motivation for this change?
>
> I was wondering why clang (CodeGen) needed the help of a command line option 
> to decide whether vec3 should be converted to vec4. Can it just preserve vec3 
> when the architecture is spir?


Ah, sorry... I forgot to link the discussion cfe-dev. 
http://lists.llvm.org/pipermail/cfe-dev/2017-March/052956.html I hope it is 
helpful for you.


https://reviews.llvm.org/D30810



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

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

[Haojian Wu via Phabricator via cfe-commits]

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

Looks good from my side.


https://reviews.llvm.org/D30720



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

[PATCH] D30792: Use callback for internalizing linked symbols.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

In https://reviews.llvm.org/D30792#697802, @mehdi_amini wrote:

> Off topic, but since this is a rev lock change with LLVM, you can to all of 
> in a single revision with: 
> http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo


Thanks! I've updated https://reviews.llvm.org/D30738 to include both changes.


Repository:
  rL LLVM

https://reviews.llvm.org/D30792



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

r297606 - clang-format: [JS] allow breaking after non-null assertions.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Mon Mar 13 04:14:23 2017
New Revision: 297606

URL: http://llvm.org/viewvc/llvm-project?rev=297606&view=rev
Log:
clang-format: [JS] allow breaking after non-null assertions.

Summary:
Previously clang-format would not break after any !. However in TypeScript, ! 
can be used as a post fix operator for non-nullability:
x.foo()!.bar()!;

With this change, clang-format will wrap after the ! if it is likely a post-fix 
non null operator.

Reviewers: djasper

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=297606&r1=297605&r2=297606&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Mon Mar 13 04:14:23 2017
@@ -54,6 +54,7 @@ namespace format {
   TYPE(JavaAnnotation) \
   TYPE(JsComputedPropertyName) \
   TYPE(JsFatArrow) \
+  TYPE(JsNonNullAssertion) \
   TYPE(JsTypeColon) \
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=297606&r1=297605&r2=297606&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Mar 13 04:14:23 2017
@@ -1030,6 +1030,23 @@ private:
   // The token type is already known.
   return;
 
+if (Style.Language == FormatStyle::LK_JavaScript) {
+  if (Current.is(tok::exclaim)) {
+if (Current.Previous &&
+(Current.Previous->isOneOf(tok::identifier, tok::r_paren,
+   tok::r_square, tok::r_brace) ||
+ Current.Previous->Tok.isLiteral())) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+if (Current.Next &&
+Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+  }
+}
+
 // Line.MightBeFunctionDecl can only be true after the parentheses of a
 // function declaration have been found. In this case, 'Current' is a
 // trailing token of this declaration and thus cannot be a name.
@@ -2284,12 +2301,9 @@ bool TokenAnnotator::spaceRequiredBefore
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
-// Postfix non-null assertion operator, as in `foo!.bar()`.
-if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
-tok::r_square, tok::r_brace) ||
-   Left.Tok.isLiteral()))
+if (Right.is(TT_JsNonNullAssertion))
   return false;
-if (Left.is(tok::exclaim) && Right.is(Keywords.kw_as))
+if (Left.is(TT_JsNonNullAssertion) && Right.is(Keywords.kw_as))
   return true; // "x! as string"
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
@@ -2545,6 +2559,8 @@ bool TokenAnnotator::canBreakBefore(cons
   return false; // must not break before as in 'x as type' casts
 if (Left.is(Keywords.kw_as))
   return true;
+if (Left.is(TT_JsNonNullAssertion))
+  return true;
 if (Left.is(Keywords.kw_declare) &&
 Right.isOneOf(Keywords.kw_module, tok::kw_namespace,
   Keywords.kw_function, tok::kw_class, tok::kw_enum,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=297606&r1=297605&r2=297606&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Mar 13 04:14:23 2017
@@ -1714,6 +1714,12 @@ TEST_F(FormatTestJS, NonNullAssertionOpe
   verifyFormat("let x = (foo)!;\n");
   verifyFormat("let x = foo! - 1;\n");
   verifyFormat("let x = {foo: 1}!;\n");
+  verifyFormat(
+  "let x = hello.foo()!\n"
+  ".foo()!\n"
+  ".foo()!\n"
+  ".foo()!;\n",
+  getGoogleJSStyleWithColumns(20));
 }
 
 TEST_F(FormatTestJS, Conditional) {


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

[PATCH] D30705: clang-format: [JS] allow breaking after non-null assertions.

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297606: clang-format: [JS] allow breaking after non-null 
assertions. (authored by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D30705?vs=90988&id=91526#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30705

Files:
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1030,6 +1030,23 @@
   // The token type is already known.
   return;
 
+if (Style.Language == FormatStyle::LK_JavaScript) {
+  if (Current.is(tok::exclaim)) {
+if (Current.Previous &&
+(Current.Previous->isOneOf(tok::identifier, tok::r_paren,
+   tok::r_square, tok::r_brace) ||
+ Current.Previous->Tok.isLiteral())) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+if (Current.Next &&
+Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+  }
+}
+
 // Line.MightBeFunctionDecl can only be true after the parentheses of a
 // function declaration have been found. In this case, 'Current' is a
 // trailing token of this declaration and thus cannot be a name.
@@ -2284,12 +2301,9 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   return false;
-// Postfix non-null assertion operator, as in `foo!.bar()`.
-if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren,
-tok::r_square, tok::r_brace) ||
-   Left.Tok.isLiteral()))
+if (Right.is(TT_JsNonNullAssertion))
   return false;
-if (Left.is(tok::exclaim) && Right.is(Keywords.kw_as))
+if (Left.is(TT_JsNonNullAssertion) && Right.is(Keywords.kw_as))
   return true; // "x! as string"
   } else if (Style.Language == FormatStyle::LK_Java) {
 if (Left.is(tok::r_square) && Right.is(tok::l_brace))
@@ -2545,6 +2559,8 @@
   return false; // must not break before as in 'x as type' casts
 if (Left.is(Keywords.kw_as))
   return true;
+if (Left.is(TT_JsNonNullAssertion))
+  return true;
 if (Left.is(Keywords.kw_declare) &&
 Right.isOneOf(Keywords.kw_module, tok::kw_namespace,
   Keywords.kw_function, tok::kw_class, tok::kw_enum,
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -54,6 +54,7 @@
   TYPE(JavaAnnotation) \
   TYPE(JsComputedPropertyName) \
   TYPE(JsFatArrow) \
+  TYPE(JsNonNullAssertion) \
   TYPE(JsTypeColon) \
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1714,6 +1714,12 @@
   verifyFormat("let x = (foo)!;\n");
   verifyFormat("let x = foo! - 1;\n");
   verifyFormat("let x = {foo: 1}!;\n");
+  verifyFormat(
+  "let x = hello.foo()!\n"
+  ".foo()!\n"
+  ".foo()!\n"
+  ".foo()!;\n",
+  getGoogleJSStyleWithColumns(20));
 }
 
 TEST_F(FormatTestJS, Conditional) {


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -1030,6 +1030,23 @@
   // The token type is already known.
   return;
 
+if (Style.Language == FormatStyle::LK_JavaScript) {
+  if (Current.is(tok::exclaim)) {
+if (Current.Previous &&
+(Current.Previous->isOneOf(tok::identifier, tok::r_paren,
+   tok::r_square, tok::r_brace) ||
+ Current.Previous->Tok.isLiteral())) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+if (Current.Next &&
+Current.Next->isOneOf(TT_BinaryOperator, Keywords.kw_as)) {
+  Current.Type = TT_JsNonNullAssertion;
+  return;
+}
+  }
+}
+
 // Line.MightBeFunctionDecl can only be true after the parentheses of a
 // function declaration have been found. In this case, 'Current' is a
 // trailing token of this declaration and thus cannot be a name.
@@ -2284,12 +2301,9 @@
   // locations that should have whitespace following are identified by the
   // above set of follower tokens.
   retur

[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Zoltan,
Thank you for the patch. There is an inline comment.




Comment at: lib/AST/ASTImporter.cpp:5221
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
-  return nullptr;
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt

In such cases, we should check that FromIdentifier is `nullptr` too (to detect 
cases where the result is `nullptr` due to import error). The check will be 
still present but will look like:
```
if (!ToII && S->getOutputIdentifier())
  return nullptr;
```
The same below.


https://reviews.llvm.org/D30831



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

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
 CheckFactories.registerCheck("misc-misplaced-const");

aaron.ballman wrote:
> leanil wrote:
> > aaron.ballman wrote:
> > > I wonder if the name might be slightly misleading -- I've always 
> > > understood these to be universal references rather than forwarding 
> > > references. I don't have the Meyers book in front of me -- what 
> > > nomenclature does he use?
> > > 
> > > Once we pick the name, we should use it consistently in the source code 
> > > (like the file name for the check and the check name), the documentation, 
> > > and the release notes.
> > Meyers calls them universal references, but it's //forwarding reference// 
> > in the standard (14.8.2.1).
> Hmm, the terms are a bit too new to really get a good idea from google's 
> ngram viewer, but the search result counts are:
> 
> Google:
> "universal reference" : 270,000
> "forwarding reference" : 9650
> 
> Stack Overflow:
> universal reference : 3016
> forwarding reference: 1654
> 
> So I think that these are probably more well-known as universal references, 
> despite the standard's nomenclature being forwarding reference.
The Q&A section in https://isocpp.org/files/papers/N4164.pdf explains why 
"universal reference" is a bad name.


Repository:
  rL LLVM

https://reviews.llvm.org/D30547



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

[PATCH] D30883: clang-format: [JS] do not wrap @see tags.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
Herald added a subscriber: klimek.

@see is special among JSDoc tags in that it is commonly followed by URLs. The 
JSDoc spec suggests that users should wrap URLs in an additional {@link url...} 
tag (@see http://usejsdoc.org/tags-see.html), but this is very commonly 
violated, with @see being followed by a "naked" URL.

This change special cases all JSDoc lines that contain an @see not to be 
wrapped to account for that.


https://reviews.llvm.org/D30883

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


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1611,6 +1611,13 @@
" * @param {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  verifyFormat("/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   "/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat(
   "/**\n"
   " * @param This is a\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -624,8 +624,9 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, and @tag followed by { for a lot of JSDoc tags.
-GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{))";
+// taze:, @tag followed by { for a lot of JSDoc tags, and @see, which is
+// commonly followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1611,6 +1611,13 @@
" * @param {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  verifyFormat("/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   "/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat(
   "/**\n"
   " * @param This is a\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -624,8 +624,9 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, and @tag followed by { for a lot of JSDoc tags.
-GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{))";
+// taze:, @tag followed by { for a lot of JSDoc tags, and @see, which is
+// commonly followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
 CheckFactories.registerCheck("misc-misplaced-const");

malcolm.parsons wrote:
> aaron.ballman wrote:
> > leanil wrote:
> > > aaron.ballman wrote:
> > > > I wonder if the name might be slightly misleading -- I've always 
> > > > understood these to be universal references rather than forwarding 
> > > > references. I don't have the Meyers book in front of me -- what 
> > > > nomenclature does he use?
> > > > 
> > > > Once we pick the name, we should use it consistently in the source code 
> > > > (like the file name for the check and the check name), the 
> > > > documentation, and the release notes.
> > > Meyers calls them universal references, but it's //forwarding reference// 
> > > in the standard (14.8.2.1).
> > Hmm, the terms are a bit too new to really get a good idea from google's 
> > ngram viewer, but the search result counts are:
> > 
> > Google:
> > "universal reference" : 270,000
> > "forwarding reference" : 9650
> > 
> > Stack Overflow:
> > universal reference : 3016
> > forwarding reference: 1654
> > 
> > So I think that these are probably more well-known as universal references, 
> > despite the standard's nomenclature being forwarding reference.
> The Q&A section in https://isocpp.org/files/papers/N4164.pdf explains why 
> "universal reference" is a bad name.
I think in a compiler related project at least in the source code we should 
stick the the standard's nomenclature. In the documentation, however, it is 
worth to mention that, this very same thing is called universal reference by 
Scott Meyers. 


Repository:
  rL LLVM

https://reviews.llvm.org/D30547



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

[PATCH] D30883: clang-format: [JS] do not wrap @see tags.

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D30883



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

r297607 - clang-format: [JS] do not wrap @see tags.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Mon Mar 13 04:39:23 2017
New Revision: 297607

URL: http://llvm.org/viewvc/llvm-project?rev=297607&view=rev
Log:
clang-format: [JS] do not wrap @see tags.

Summary:
@see is special among JSDoc tags in that it is commonly followed by URLs. The 
JSDoc spec suggests that users should wrap URLs in an additional {@link url...} 
tag (@see http://usejsdoc.org/tags-see.html), but this is very commonly 
violated, with @see being followed by a "naked" URL.

This change special cases all JSDoc lines that contain an @see not to be 
wrapped to account for that.

Reviewers: djasper

Subscribers: klimek, cfe-commits

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

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

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=297607&r1=297606&r2=297607&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Mar 13 04:39:23 2017
@@ -624,8 +624,9 @@ FormatStyle getGoogleStyle(FormatStyle::
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, and @tag followed by { for a lot of JSDoc tags.
-GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{))";
+// taze:, @tag followed by { for a lot of JSDoc tags, and @see, which is
+// commonly followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=297607&r1=297606&r2=297607&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon Mar 13 04:39:23 2017
@@ -1611,6 +1611,13 @@ TEST_F(FormatTestJS, JSDocAnnotations) {
" * @param {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  verifyFormat("/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   "/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat(
   "/**\n"
   " * @param This is a\n"


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

[PATCH] D30883: clang-format: [JS] do not wrap @see tags.

[Martin Probst via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297607: clang-format: [JS] do not wrap @see tags. (authored 
by mprobst).

Changed prior to commit:
  https://reviews.llvm.org/D30883?vs=91527&id=91529#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30883

Files:
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -624,8 +624,9 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, and @tag followed by { for a lot of JSDoc tags.
-GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{))";
+// taze:, @tag followed by { for a lot of JSDoc tags, and @see, which is
+// commonly followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1611,6 +1611,13 @@
" * @param {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  verifyFormat("/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   "/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat(
   "/**\n"
   " * @param This is a\n"


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -624,8 +624,9 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
-// taze:, and @tag followed by { for a lot of JSDoc tags.
-GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{))";
+// taze:, @tag followed by { for a lot of JSDoc tags, and @see, which is
+// commonly followed by overlong URLs.
+GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]+[ \\t]*{)|@see)";
 GoogleStyle.MaxEmptyLinesToKeep = 3;
 GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
 GoogleStyle.SpacesInContainerLiterals = false;
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -1611,6 +1611,13 @@
" * @param {this.is.a.long.path.to.a.Type}\n"
" */",
getGoogleJSStyleWithColumns(20));
+  verifyFormat("/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   "/**\n"
+   " * @see http://very/very/long/url/is/long\n";
+   " */",
+   getGoogleJSStyleWithColumns(20));
   verifyFormat(
   "/**\n"
   " * @param This is a\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30860: [clang-format] Add more examples and fix a bug in the py generation script

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.




Comment at: docs/tools/dump_format_style.py:67
   def __str__(self):
-return '* ``%s`` %s' % (self.name, doxygen2rst(self.comment))
+return '\n* ``%s`` %s' % (self.name, doxygen2rst(self.comment))
 

sylvestre.ledru wrote:
> This is needed to fix a bug in the generation of BraceWrappingFlags
> The generation was failing because of a missing empty line with:
> 
> 
> 
> > docs/ClangFormatStyleOptions.rst:480: (WARNING/2) Explicit markup ends 
> > without a blank line; unexpected unindent.
> 
> 
> Let me know if you prefer to see that in a separate patch
> 
The same patch is fine. Thanks for fixing this!


https://reviews.llvm.org/D30860



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

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+  "initialization of atomic variables is restricted to variables in global 
address space">;
 def err_atomic_init_constant : Error<

Anastasia wrote:
> echuraev wrote:
> > Anastasia wrote:
> > > Could we combine this error diag with the one below? I guess they are 
> > > semantically very similar apart from one is about initialization and 
> > > another is about assignment?
> > I'm not sure that it is a good idea to combine these errors. For example, 
> > if developer had declared a variable non-constant and not in global address 
> > space he would have got the same message for both errors. And it can be 
> > difficult to determine what the exact problem is. He can fix one of the 
> > problems but he will still get the same error.
> Well, I don't actually see that we check for constant anywhere so it's also 
> OK if you want to drop this bit. Although I think the original intension of 
> this message as I understood was to provide the most complete hint.
> 
> My concern is that these two errors seem to be reporting nearly the same 
> issue and ideally we would like to keep diagnostic list as small as possible. 
> This also makes the file more concise and messages more consistent.
I suggest adding a test case with non-constant initialization case to validate 
that existing checks cover this case for atomic types already.
If so, we can adjust existing diagnostic message to cover both cases: 
initialization and assignment expression.


https://reviews.llvm.org/D30643



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

RE: r297468 - [OpenCL] Fix type compatibility check and generic AS mangling.

[Anastasia Stulova via cfe-commits]

Hi Galina,

Thanks for reporting it! I believe the issue is fixed in r297530.

Cheers,
Anastasia

From: Galina Kistanova [mailto:gkistan...@gmail.com]
Sent: 10 March 2017 20:29
To: Anastasia Stulova
Cc: cfe-commits
Subject: Re: r297468 - [OpenCL] Fix type compatibility check and generic AS 
mangling.

Hello Anastasia,

Added test fails on one of our new builders:

Failing Tests (1):
Clang :: 
SemaOpenCL/overload_addrspace_resolution.cl

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/23/steps/test-check-all/logs/stdio

Please have a look at it?

Thanks

Galina

On Fri, Mar 10, 2017 at 7:23 AM, Anastasia Stulova via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: stulova
Date: Fri Mar 10 09:23:07 2017
New Revision: 297468

URL: http://llvm.org/viewvc/llvm-project?rev=297468&view=rev
Log:
[OpenCL] Fix type compatibility check and generic AS mangling.

1. Reimplemented conditional operator so that it checks
compatibility of unqualified pointees of the 2nd and
the 3rd operands (C99, OpenCL v2.0 6.5.15).

Define QualTypes compatibility for OpenCL as following:

   - corresponding types are compatible (C99 6.7.3)
   - CVR-qualifiers are equal (C99 6.7.3)
   - address spaces are equal (implementation defined)

2. Added generic address space to Itanium mangling.

Review: D30037

Patch by Dmitry Borisenkov!


Added:

cfe/trunk/test/SemaOpenCL/overload_addrspace_resolution.cl
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp

cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=297468&r1=297467&r2=297468&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Mar 10 09:23:07 2017
@@ -8066,15 +8066,6 @@ QualType ASTContext::mergeTypes(QualType
   Qualifiers LQuals = LHSCan.getLocalQualifiers();
   Qualifiers RQuals = RHSCan.getLocalQualifiers();
   if (LQuals != RQuals) {
-if (getLangOpts().OpenCL) {
-  if (LHSCan.getUnqualifiedType() != RHSCan.getUnqualifiedType() ||
-  LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers())
-return QualType();
-  if (LQuals.isAddressSpaceSupersetOf(RQuals))
-return LHS;
-  if (RQuals.isAddressSpaceSupersetOf(LQuals))
-return RHS;
-}
 // If any of these qualifiers are different, we have a type
 // mismatch.
 if (LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers() ||
@@ -8200,6 +8191,20 @@ QualType ASTContext::mergeTypes(QualType
   LHSPointee = LHSPointee.getUnqualifiedType();
   RHSPointee = RHSPointee.getUnqualifiedType();
 }
+if (getLangOpts().OpenCL) {
+  Qualifiers LHSPteeQual = LHSPointee.getQualifiers();
+  Qualifiers RHSPteeQual = RHSPointee.getQualifiers();
+  // Blocks can't be an expression in a ternary operator (OpenCL v2.0
+  // 6.12.5) thus the following check is asymmetric.
+  if (!LHSPteeQual.isAddressSpaceSupersetOf(RHSPteeQual))
+return QualType();
+  LHSPteeQual.removeAddressSpace();
+  RHSPteeQual.removeAddressSpace();
+  LHSPointee =
+  QualType(LHSPointee.getTypePtr(), LHSPteeQual.getAsOpaqueValue());
+  RHSPointee =
+  QualType(RHSPointee.getTypePtr(), RHSPteeQual.getAsOpaqueValue());
+}
 QualType ResultType = mergeTypes(LHSPointee, RHSPointee, OfBlockPointer,
  Unqualified);
 if (ResultType.isNull()) return QualType();

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=297468&r1=297467&r2=297468&view=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Fri Mar 10 09:23:07 2017
@@ -2159,10 +2159,12 @@ void CXXNameMangler::mangleQualifiers(Qu
 } else {
   switch (AS) {
   default: llvm_unreachable("Not a language specific address space");
-  //   ::= "CL" [ "global" | "local" | "constant" ]
+  //   ::= "CL" [ "global" | "local" | "constant |
+  //"generic" ]
   case LangAS::opencl_global:   ASString = "CLglobal";   break;
   case LangAS::opencl_local:ASString = "CLlocal";break;
   case LangAS::opencl_constant: ASString = "CLconstant"; break;
+  case LangAS::opencl_generic:  ASString = "CLgeneric";  break;
   //   ::= "CU" [ "device" | "constant" | "shared" ]
   case LangAS::cuda_device: ASString = "CUdevice";   break;
   case LangAS::cuda_constant:   ASString = "CUconstant"; break

[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.

Given that we have already explicitly stated in the qualifier that the 
expression is `__unaligned`,
it makes little sense to diagnose that the address of the packed member may not 
be aligned.


https://reviews.llvm.org/D30884

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/address-unaligned.c


Index: test/Sema/address-unaligned.c
===
--- /dev/null
+++ test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(&s1->x);
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;


Index: test/Sema/address-unaligned.c
===
--- /dev/null
+++ test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(&s1->x);
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r297614 - Add -iframeworkwithsysroot compiler option

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Mar 13 06:17:41 2017
New Revision: 297614

URL: http://llvm.org/viewvc/llvm-project?rev=297614&view=rev
Log:
Add -iframeworkwithsysroot compiler option

This commit adds support for a new -iframeworkwithsysroot compiler option which
allows the user to specify a framework path that can be prefixed with the
sysroot. This option is similar to the -iwithsysroot option that exists to
supplement -isystem.

rdar://21316352

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Frontend/iframework.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=297614&r1=297613&r2=297614&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Mar 13 06:17:41 2017
@@ -1524,6 +1524,11 @@ def idirafter : JoinedOrSeparate<["-"],
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, 
Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=297614&r1=297613&r2=297614&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Mar 13 06:17:41 2017
@@ -1507,6 +1507,9 @@ static void ParseHeaderSearchArgs(Header
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))

Modified: cfe/trunk/test/Frontend/iframework.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/iframework.c?rev=297614&r1=297613&r2=297614&view=diff
==
--- cfe/trunk/test/Frontend/iframework.c (original)
+++ cfe/trunk/test/Frontend/iframework.c Mon Mar 13 06:17:41 2017
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s 
-Xclang -verify
 // expected-no-diagnostics
 
 #include 


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

[PATCH] D30183: Add -iframeworkwithsysroot compiler option

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297614: Add -iframeworkwithsysroot compiler option (authored 
by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D30183?vs=89152&id=91534#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30183

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Frontend/iframework.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, 
Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, 
Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;
Index: cfe/trunk/test/Frontend/iframework.c
===
--- cfe/trunk/test/Frontend/iframework.c
+++ cfe/trunk/test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s 
-Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1507,6 +1507,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1524,6 +1524,11 @@
   HelpText<"Add directory to AFTER include search path">;
 def iframework : JoinedOrSeparate<["-"], "iframework">, Group, Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM framework search path">;
+def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
+  Group,
+  HelpText<"Add directory to SYSTEM framework search path, "
+   "absolute paths are relative to -isysroot">,
+  MetaVarName<"">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group, Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"">;
 def image__base : Separate<["-"], "image_base">;
Index: cfe/trunk/test/Frontend/iframework.c
===
--- cfe/trunk/test/Frontend/iframework.c
+++ cfe/trunk/test/Frontend/iframework.c
@@ -1,4 +1,5 @@
 // RUN: %clang -fsyntax-only -iframework %S/Inputs %s -Xclang -verify
+// RUN: %clang -fsyntax-only -isysroot %S -iframeworkwithsysroot /Inputs %s -Xclang -verify
 // expected-no-diagnostics
 
 #include 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1507,6 +1507,9 @@
  !A->getOption().matches(OPT_iwithsysroot));
   for (const Arg *A : Args.filtered(OPT_iframework))
 Opts.AddPath(A->getValue(), frontend::System, true, true);
+  for (const Arg *A : Args.filtered(OPT_iframeworkwithsysroot))
+Opts.AddPath(A->getValue(), frontend::System, /*IsFramework=*/true,
+ /*IgnoreSysRoot=*/false);
 
   // Add the paths for the various language specific isystem flags.
   for (const Arg *A : Args.filtered(OPT_c_isystem))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
 CheckFactories.registerCheck("misc-misplaced-const");

xazax.hun wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > leanil wrote:
> > > > aaron.ballman wrote:
> > > > > I wonder if the name might be slightly misleading -- I've always 
> > > > > understood these to be universal references rather than forwarding 
> > > > > references. I don't have the Meyers book in front of me -- what 
> > > > > nomenclature does he use?
> > > > > 
> > > > > Once we pick the name, we should use it consistently in the source 
> > > > > code (like the file name for the check and the check name), the 
> > > > > documentation, and the release notes.
> > > > Meyers calls them universal references, but it's //forwarding 
> > > > reference// in the standard (14.8.2.1).
> > > Hmm, the terms are a bit too new to really get a good idea from google's 
> > > ngram viewer, but the search result counts are:
> > > 
> > > Google:
> > > "universal reference" : 270,000
> > > "forwarding reference" : 9650
> > > 
> > > Stack Overflow:
> > > universal reference : 3016
> > > forwarding reference: 1654
> > > 
> > > So I think that these are probably more well-known as universal 
> > > references, despite the standard's nomenclature being forwarding 
> > > reference.
> > The Q&A section in https://isocpp.org/files/papers/N4164.pdf explains why 
> > "universal reference" is a bad name.
> I think in a compiler related project at least in the source code we should 
> stick the the standard's nomenclature. In the documentation, however, it is 
> worth to mention that, this very same thing is called universal reference by 
> Scott Meyers. 
Okay, I'm convinced enough that "forward reference" is okay. :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D30547



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

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Egor Churaev via Phabricator via cfe-commits]

echuraev updated this revision to Diff 91536.

https://reviews.llvm.org/D30643

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  test/Parser/opencl-atomics-cl20.cl
  test/SemaOpenCL/atomic-init.cl


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant or to variables in global adress space}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can only be assigned 
to a compile time constant or to variables in global adress space}}
+  private atomic_int a3 = 0; // expected-error {{atomic variable can only be 
assigned to a compile time constant or to variables in global adress space}}
+  local atomic_int a4 = 0; // expected-error {{'__local' variable cannot have 
an initializer}}
+  global atomic_int a5 = 0; // expected-error {{function scope variable cannot 
be declared in global address space}}
+}
Index: test/Parser/opencl-atomics-cl20.cl
===
--- test/Parser/opencl-atomics-cl20.cl
+++ test/Parser/opencl-atomics-cl20.cl
@@ -67,7 +67,7 @@
   foo(&i);
 // OpenCL v2.0 s6.13.11.8, arithemtic operations are not permitted on atomic 
types.
   i++; // expected-error {{invalid argument type 'atomic_int' (aka 
'_Atomic(int)') to unary expression}}
-  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant in the declaration statement in the program scope}}
+  i = 1; // expected-error {{atomic variable can only be assigned to a compile 
time constant or to variables in global adress space}}
   i += 1; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'int')}}
   i = i + i; // expected-error {{invalid operands to binary expression 
('atomic_int' (aka '_Atomic(int)') and 'atomic_int')}}
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6489,6 +6489,20 @@
   << Init->getSourceRange();
   }
 
+  // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global 
scope
+  QualType ETy = Entity.getType();
+  Qualifiers TyQualifiers = ETy.getQualifiers();
+  bool HasGlobalAS = TyQualifiers.hasAddressSpace() &&
+ TyQualifiers.getAddressSpace() == LangAS::opencl_global;
+
+  if (S.getLangOpts().OpenCLVersion >= 200 &&
+  ETy->isAtomicType() && !HasGlobalAS &&
+  Entity.getKind() == InitializedEntity::EK_Variable && Args.size() > 0) {
+S.Diag(Args[0]->getLocStart(), diag::err_atomic_init) <<
+SourceRange(Entity.getDecl()->getLocStart(), Args[0]->getLocEnd());
+return ExprError();
+  }
+
   // Diagnose cases where we initialize a pointer to an array temporary, and 
the
   // pointer obviously outlives the temporary.
   if (Args.size() == 1 && Args[0]->getType()->isArrayType() &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11091,7 +11091,7 @@
 if (LHSTy->isAtomicType() || RHSTy->isAtomicType()) {
   SourceRange SR(LHSExpr->getLocStart(), RHSExpr->getLocEnd());
   if (BO_Assign == Opc)
-Diag(OpLoc, diag::err_atomic_init_constant) << SR;
+Diag(OpLoc, diag::err_atomic_init) << SR;
   else
 ResultTy = InvalidOperands(OpLoc, LHS, RHS);
   return ExprError();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8264,9 +8264,9 @@
   "return value cannot be qualified with address space">;
 def err_opencl_constant_no_init : Error<
   "variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
-  "atomic variable can only be assigned to a compile time constant"
-  " in the declaration statement in the program scope">;
+// Atomics
+def err_atomic_init: Error<
+  "atomic variable can only be assigned to a compile time constant or to 
variables in global adress space">;
 def err_opencl_implicit_vector_conversion : Error<
   "implicit conversions between vector types (%0 and %1) are not permitted">;
 def err_opencl_invalid_type_array : Error<


Index: test/SemaOpenCL/atomic-init.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-init.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -cl-std=CL2.0 -fsyntax-only -verify  %s
+
+global atomic_int a1 = 0;
+
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic vari

[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D30884



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

[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the review @aaron.ballman!


https://reviews.llvm.org/D30884



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Mads Ravn via Phabricator via cfe-commits]

madsravn added a comment.

In https://reviews.llvm.org/D30158#698871, @aaron.ballman wrote:

> In https://reviews.llvm.org/D30158#696534, @madsravn wrote:
>
> > Any updates on this?
>
>
> Have you run it over the test suite on the revision before random_shuffle was 
> removed from libc++?


I can't find any more tests for random_shuffle. I have looked in the SVN log 
for from december 2014 until now. It works on the three tests currently in 
trunk.


https://reviews.llvm.org/D30158



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

[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Zoltán Gera via Phabricator via cfe-commits]

gerazo updated this revision to Diff 91541.
gerazo added a comment.

Better check not letting a real import problem passing through


https://reviews.llvm.org/D30831

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/asm/Inputs/asm-function.cpp
  test/ASTMerge/asm/test.cpp


Index: test/ASTMerge/asm/test.cpp
===
--- test/ASTMerge/asm/test.cpp
+++ test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- test/ASTMerge/asm/Inputs/asm-function.cpp
+++ test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }


Index: test/ASTMerge/asm/test.cpp
===
--- test/ASTMerge/asm/test.cpp
+++ test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- test/ASTMerge/asm/Inputs/asm-function.cpp
+++ test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+  "initialization of atomic variables is restricted to variables in global 
address space">;
 def err_atomic_init_constant : Error<

bader wrote:
> Anastasia wrote:
> > echuraev wrote:
> > > Anastasia wrote:
> > > > Could we combine this error diag with the one below? I guess they are 
> > > > semantically very similar apart from one is about initialization and 
> > > > another is about assignment?
> > > I'm not sure that it is a good idea to combine these errors. For example, 
> > > if developer had declared a variable non-constant and not in global 
> > > address space he would have got the same message for both errors. And it 
> > > can be difficult to determine what the exact problem is. He can fix one 
> > > of the problems but he will still get the same error.
> > Well, I don't actually see that we check for constant anywhere so it's also 
> > OK if you want to drop this bit. Although I think the original intension of 
> > this message as I understood was to provide the most complete hint.
> > 
> > My concern is that these two errors seem to be reporting nearly the same 
> > issue and ideally we would like to keep diagnostic list as small as 
> > possible. This also makes the file more concise and messages more 
> > consistent.
> I suggest adding a test case with non-constant initialization case to 
> validate that existing checks cover this case for atomic types already.
> If so, we can adjust existing diagnostic message to cover both cases: 
> initialization and assignment expression.
I don't think it's quite true.
There are two requirements here that must be met the same time. Atomic 
variables *declared in the global address space* can be initialized only with 
"compile time constant'.
If understand the spec correctly this code is also valid:

  kernel void foo() {
static global atomic_int a = 42; // although it's not clear if we must use 
ATOMIC_VAR_INIT here.
...
  }



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8267
   "variable in constant address space must be initialized">;
-def err_atomic_init_constant : Error<
-  "atomic variable can only be assigned to a compile time constant"
-  " in the declaration statement in the program scope">;
+// Atomics
+def err_atomic_init: Error<

I suggest removing this comment.
If you are going to add other diagnostic messages specific to OpenCL atomics, 
then separate them from this list of unordered diagnostics similar to pipe 
built-in functions below.


https://reviews.llvm.org/D30643



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

[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Zoltán Gera via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTImporter.cpp:5221
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
-  return nullptr;
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt

a.sidorin wrote:
> In such cases, we should check that FromIdentifier is `nullptr` too (to 
> detect cases where the result is `nullptr` due to import error). The check 
> will be still present but will look like:
> ```
> if (!ToII && S->getOutputIdentifier())
>   return nullptr;
> ```
> The same below.
Thank you Aleksei, done.


https://reviews.llvm.org/D30831



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

r297619 - [analyzer] Fix a rare crash for valist check.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Mar 13 07:48:26 2017
New Revision: 297619

URL: http://llvm.org/viewvc/llvm-project?rev=297619&view=rev
Log:
[analyzer] Fix a rare crash for valist check.

It looks like on some host-triples the result of a valist related expr can be
a LazyCompoundVal. Handle that case in the check.

Patch by Abramo Bagnara!

Added:
cfe/trunk/test/Analysis/valist-as-lazycompound.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp?rev=297619&r1=297618&r2=297619&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp Mon Mar 13 07:48:26 
2017
@@ -165,11 +165,8 @@ void ValistChecker::checkPreCall(const C
 const MemRegion *ValistChecker::getVAListAsRegion(SVal SV, const Expr *E,
   bool &IsSymbolic,
   CheckerContext &C) const {
-  // FIXME: on some platforms CallAndMessage checker finds some instances of
-  // the uninitialized va_list usages. CallAndMessage checker is disabled in
-  // the tests so they can verify platform independently those issues. As a
-  // side effect, this check is required here.
-  if (SV.isUnknownOrUndef())
+  const MemRegion *Reg = SV.getAsRegion();
+  if (!Reg)
 return nullptr;
   // TODO: In the future this should be abstracted away by the analyzer.
   bool VaListModelledAsArray = false;
@@ -178,7 +175,6 @@ const MemRegion *ValistChecker::getVALis
 VaListModelledAsArray =
 Ty->isPointerType() && Ty->getPointeeType()->isRecordType();
   }
-  const MemRegion *Reg = SV.getAsRegion();
   if (const auto *DeclReg = Reg->getAs()) {
 if (isa(DeclReg->getDecl()))
   Reg = C.getState()->getSVal(SV.castAs()).getAsRegion();

Added: cfe/trunk/test/Analysis/valist-as-lazycompound.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/valist-as-lazycompound.c?rev=297619&view=auto
==
--- cfe/trunk/test/Analysis/valist-as-lazycompound.c (added)
+++ cfe/trunk/test/Analysis/valist-as-lazycompound.c Mon Mar 13 07:48:26 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -triple gcc-linaro-arm-linux-gnueabihf 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
+// expected-no-diagnostics
+
+typedef unsigned int size_t;
+typedef __builtin_va_list __gnuc_va_list;
+typedef __gnuc_va_list va_list;
+
+extern int vsprintf(char *__restrict __s,
+const char *__restrict __format, __gnuc_va_list
+ __arg);
+
+void _dprintf(const char *function, int flen, int line, int level,
+ const char *prefix, const char *fmt, ...) {
+  char raw[10];
+  int err;
+  va_list ap;
+
+  __builtin_va_start(ap, fmt);
+  err = vsprintf(raw, fmt, ap);
+  __builtin_va_end(ap);
+}

Modified: cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c?rev=297619&r1=297618&r2=297619&view=diff
==
--- cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c (original)
+++ cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c Mon Mar 13 07:48:26 
2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu 
-analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf 
-analyzer-output=text -analyzer-store=region -verify %s
 
 #include "Inputs/system-header-simulator-for-valist.h"
 


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

r297620 - When diagnosing taking address of packed members skip __unaligned-qualified expressions

[Roger Ferrer Ibanez via cfe-commits]

Author: rogfer01
Date: Mon Mar 13 08:18:21 2017
New Revision: 297620

URL: http://llvm.org/viewvc/llvm-project?rev=297620&view=rev
Log:
When diagnosing taking address of packed members skip __unaligned-qualified 
expressions

Given that we have already explicitly stated in the qualifier that the
expression is __unaligned, it makes little sense to diagnose that the address
of the packed member may not be aligned.

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


Added:
cfe/trunk/test/Sema/address-unaligned.c
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=297620&r1=297619&r2=297620&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Mar 13 08:18:21 2017
@@ -11851,6 +11851,10 @@ void Sema::RefersToMemberWithReducedAlig
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;

Added: cfe/trunk/test/Sema/address-unaligned.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/address-unaligned.c?rev=297620&view=auto
==
--- cfe/trunk/test/Sema/address-unaligned.c (added)
+++ cfe/trunk/test/Sema/address-unaligned.c Mon Mar 13 08:18:21 2017
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(&s1->x);
+}


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

[PATCH] D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297620: When diagnosing taking address of packed members 
skip __unaligned-qualified… (authored by rogfer01).

Changed prior to commit:
  https://reviews.llvm.org/D30884?vs=91531&id=91548#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30884

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/address-unaligned.c


Index: cfe/trunk/test/Sema/address-unaligned.c
===
--- cfe/trunk/test/Sema/address-unaligned.c
+++ cfe/trunk/test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(&s1->x);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;


Index: cfe/trunk/test/Sema/address-unaligned.c
===
--- cfe/trunk/test/Sema/address-unaligned.c
+++ cfe/trunk/test/Sema/address-unaligned.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify %s
+// expected-no-diagnostics
+
+typedef
+struct __attribute__((packed)) S1 {
+  char c0;
+  int x;
+  char c1;
+} S1;
+
+void bar(__unaligned int *);
+
+void foo(__unaligned S1* s1)
+{
+bar(&s1->x);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11851,6 +11851,10 @@
   if (!ME)
 return;
 
+  // No need to check expressions with an __unaligned-qualified type.
+  if (E->getType().getQualifiers().hasUnaligned())
+return;
+
   // For a chain of MemberExpr like "a.b.c.d" this list
   // will keep FieldDecl's like [d, c, b].
   SmallVector ReverseMemberChain;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Stanisław Barzowski via Phabricator via cfe-commits]

sbarzowski added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

sbarzowski wrote:
> vmiklos wrote:
> > Prazek wrote:
> > > aaron.ballman wrote:
> > > > What is the false positive rate with this check over a large codebase 
> > > > that uses exceptions?
> > > Do you know any good project to check it?
> > LibreOffice might be a candidate, see 
> >  for details on 
> > how to generate a compile database for it (since it does not use cmake), 
> > then you can start testing.
> Thanks. However it's not just about using exception. To be meaningful I need 
> a project that use noexcept in more than a few places.
> 
> Here are some projects that I found:
> https://github.com/facebook/hhvm/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/folly/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/Tencent/mars/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/SFTtech/openage/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/watchman/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/facebook/proxygen/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/philsquared/Catch/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93&q=noexcept
> https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93&q=noexcept
> 
> I've tried HHVM so far, and ran into some problems compiling it. Anyway I'm 
> working on it.
Ok, I was able to run it on most of the HHVM  codebase + deps. There were some 
issues that looked like some autogenerated pieces missing, so it may have been 
not 100% covered.

The results:
3 occurences in total
1) rethrow in destructor (http://pastebin.com/JRNMZGev)
2) false positive "throw and catch in the same function" 
(http://pastebin.com/14y1AJgM)
3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)

That's not too many, but this is a kind of check that I guess would be most 
useful earlier in the development - ideally before the initial code review.

I'm not sure if we should count (3) as false positive. We could potentially 
eliminate it, by checking if the expression in noexcept is empty or true 
literal.

(2) is def. a false positive. The potential handling of this case was already 
proposed, but I'm not sure if it's worth it.  The code in example (2) is ugly 
and extracting these throwing parts to separate functions looks like a good way 
to start refactoring. 

What do you think?



https://reviews.llvm.org/D19201



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

[PATCH] D29877: Warn about unused static file scope function template declarations.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@rsmith ping...


https://reviews.llvm.org/D29877



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

r297623 - [clang-format] Add more examples and fix a bug in the py generation script

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Mon Mar 13 09:42:47 2017
New Revision: 297623

URL: http://llvm.org/viewvc/llvm-project?rev=297623&view=rev
Log:
[clang-format] Add more examples and fix a bug in the py generation script

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/tools/dump_format_style.py
cfe/trunk/include/clang/Format/Format.h

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=297623&r1=297622&r2=297623&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Mar 13 09:42:47 2017
@@ -252,6 +252,13 @@ the configuration (without a prefix: ``A
   Allow putting all parameters of a function declaration onto
   the next line even if ``BinPackParameters`` is ``false``.
 
+  .. code-block:: c++
+
+true:   false:
+myFunction(foo, vs. myFunction(foo, bar, plop);
+   bar,
+   plop);
+
 **AllowShortBlocksOnASingleLine** (``bool``)
   Allows contracting simple braced statements to a single line.
 
@@ -460,16 +467,148 @@ the configuration (without a prefix: ``A
 
   Nested configuration flags:
 
+
   * ``bool AfterClass`` Wrap class definitions.
+
+  .. code-block:: c++
+
+true:
+class foo {};
+
+false:
+class foo
+{};
+
   * ``bool AfterControlStatement`` Wrap control statements 
(``if``/``for``/``while``/``switch``/..).
+
+  .. code-block:: c++
+
+true:
+if (foo())
+{
+} else
+{}
+for (int i = 0; i < 10; ++i)
+{}
+
+false:
+if (foo()) {
+} else {
+}
+for (int i = 0; i < 10; ++i) {
+}
+
   * ``bool AfterEnum`` Wrap enum definitions.
+
+  .. code-block:: c++
+
+true:
+enum X : int
+{
+  B
+};
+
+false:
+enum X : int { B };
+
   * ``bool AfterFunction`` Wrap function definitions.
+
+  .. code-block:: c++
+
+true:
+void foo()
+{
+  bar();
+  bar2();
+}
+
+false:
+void foo() {
+  bar();
+  bar2();
+}
+
   * ``bool AfterNamespace`` Wrap namespace definitions.
+
+  .. code-block:: c++
+
+true:
+namespace
+{
+int foo();
+int bar();
+}
+
+false:
+namespace {
+int foo();
+int bar();
+}
+
   * ``bool AfterObjCDeclaration`` Wrap ObjC definitions (``@autoreleasepool``, 
interfaces, ..).
+
   * ``bool AfterStruct`` Wrap struct definitions.
+
+  .. code-block:: c++
+
+true:
+struct foo
+{
+  int x;
+}
+
+false:
+struct foo {
+  int x;
+}
+
   * ``bool AfterUnion`` Wrap union definitions.
+
+  .. code-block:: c++
+
+true:
+union foo
+{
+  int x;
+}
+
+false:
+union foo {
+  int x;
+}
+
   * ``bool BeforeCatch`` Wrap before ``catch``.
+
+  .. code-block:: c++
+
+true:
+try {
+  foo();
+}
+catch () {
+}
+
+false:
+try {
+  foo();
+} catch () {
+}
+
   * ``bool BeforeElse`` Wrap before ``else``.
+
+  .. code-block:: c++
+
+true:
+if (foo()) {
+}
+else {
+}
+
+false:
+if (foo()) {
+} else {
+}
+
   * ``bool IndentBraces`` Indent the wrapped braces themselves.
 
 
@@ -500,29 +639,146 @@ the configuration (without a prefix: ``A
   * ``BS_Attach`` (in configuration: ``Attach``)
 Always attach braces to surrounding context.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo {};
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Linux`` (in configuration: ``Linux``)
 Like ``Attach``, but break before braces on function, namespace and
 class definitions.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Mozilla`` (in configuration: ``Mozilla``)
 Like ``Attach``, but break before braces on enum, function, and record
 definitions.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int { A, B };
+
   * ``BS_Stroustrup`` (in configuration: ``Stroustrup``)
 Like ``Attach``, but break before function definitions, ``catch``, and
 ``else``.
 
+.. code-block:: c++
+
+  try {
+foo();
+  } catch () {
+  }
+  void foo() { bar(); }
+  class foo
+  {
+  };
+  if (foo()) {
+  } else {
+  }
+  enum X : int
+ 

[PATCH] D29923: Add test for PPCallbacks::MacroUndefined

[Frederich Munch via Phabricator via cfe-commits]

marsupial updated this revision to Diff 91563.
marsupial retitled this revision from "Send UndefMacroDirective to MacroDefined 
callback" to "Add test for PPCallbacks::MacroUndefined".
marsupial edited the summary of this revision.

https://reviews.llvm.org/D29923

Files:
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -246,12 +246,18 @@
 namespace {
 
 struct MacroAction {
+  enum Kind { kDefinition, kUnDefinition, kExpansion};
+
   SourceLocation Loc;
   std::string Name;
-  bool isDefinition; // if false, it is expansion.
-  
-  MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-: Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+  unsigned MAKind : 2;
+
+  MacroAction(SourceLocation Loc, StringRef Name, unsigned K)
+: Loc(Loc), Name(Name), MAKind(K) { }
+
+  bool isExpansion() const { return MAKind == kExpansion; }
+  bool isDefinition() const { return MAKind == kDefinition; }
+  bool isUnDefinition() const { return MAKind == kUnDefinition; }
 };
 
 class MacroTracker : public PPCallbacks {
@@ -264,21 +270,29 @@
 const MacroDirective *MD) override {
 Macros.push_back(MacroAction(MD->getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- true));
+ MacroAction::kDefinition));
+  }
+  void MacroUndefined(const Token &MacroNameTok,
+  const MacroDefinition &MD) override {
+Macros.push_back(MacroAction(MD.getMacroInfo()->getDefinitionLoc(),
+ MacroNameTok.getIdentifierInfo()->getName(),
+ MacroAction::kUnDefinition));
   }
   void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
 SourceRange Range, const MacroArgs *Args) override {
 Macros.push_back(MacroAction(MacroNameTok.getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- false));
+ MacroAction::kExpansion));
   }
 };
 
 }
 
 TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   const char *header =
-"#define MACRO_IN_INCLUDE 0\n";
+"#define MACRO_IN_INCLUDE 0\n"
+"#define MACRO_DEFINED\n"
+"#undef MACRO_DEFINED\n";
 
   const char *main =
 "#define M(x) x\n"
@@ -323,42 +337,50 @@
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
-  ASSERT_EQ(9U, Macros.size());
+  ASSERT_EQ(13U, Macros.size());
   // #define M(x) x
-  ASSERT_TRUE(Macros[0].isDefinition);
+  ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);
   // #define INC "/test-header.h"
-  ASSERT_TRUE(Macros[1].isDefinition);
+  ASSERT_TRUE(Macros[1].isDefinition());
   ASSERT_EQ("INC", Macros[1].Name);
   // M expansion in #include M(INC)
-  ASSERT_FALSE(Macros[2].isDefinition);
+  ASSERT_FALSE(Macros[2].isDefinition());
   ASSERT_EQ("M", Macros[2].Name);
   // INC expansion in #include M(INC)
-  ASSERT_FALSE(Macros[3].isDefinition);
+  ASSERT_TRUE(Macros[3].isExpansion());
   ASSERT_EQ("INC", Macros[3].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[4].isDefinition);
+  ASSERT_TRUE(Macros[4].isDefinition());
   ASSERT_EQ("MACRO_IN_INCLUDE", Macros[4].Name);
+  // #define MACRO_DEFINED
+  ASSERT_TRUE(Macros[5].isDefinition());
+  ASSERT_FALSE(Macros[5].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[5].Name);
+  // #undef MACRO_DEFINED
+  ASSERT_FALSE(Macros[6].isDefinition());
+  ASSERT_TRUE(Macros[6].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[6].Name);
   // #define INC2 
-  ASSERT_TRUE(Macros[5].isDefinition);
-  ASSERT_EQ("INC2", Macros[5].Name);
+  ASSERT_TRUE(Macros[7].isDefinition());
+  ASSERT_EQ("INC2", Macros[7].Name);
   // M expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[6].isDefinition);
-  ASSERT_EQ("M", Macros[6].Name);
+  ASSERT_FALSE(Macros[8].isDefinition());
+  ASSERT_EQ("M", Macros[8].Name);
   // INC2 expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[7].isDefinition);
-  ASSERT_EQ("INC2", Macros[7].Name);
+  ASSERT_TRUE(Macros[9].isExpansion());
+  ASSERT_EQ("INC2", Macros[9].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[8].isDefinition);
-  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[8].Name);
+  ASSERT_TRUE(Macros[10].isDefinition());
+  ASSERT_EQ("MACRO_IN_INCLUDE", Macros[10].Name);
 
   // The INC expansion in #include M(INC) comes before the first
   // MACRO_IN_INCLUDE definition of the included file.
   EXPECT_TRUE(SourceMgr.isBeforeInTranslationUnit(Macros[3].Loc, Macros[4].Loc));
 
   // The INC2 expansion in #include M(INC2) comes before the second
   // MACRO_IN_INCLUDE definition of the included file.
-  EXPECT_TRUE(SourceMgr.isBeforeIn

[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Looks good, thank you!


https://reviews.llvm.org/D30831



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

r297627 - [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Mar 13 10:32:24 2017
New Revision: 297627

URL: http://llvm.org/viewvc/llvm-project?rev=297627&view=rev
Log:
[ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

Patch by Zoltan Gera!

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
cfe/trunk/test/ASTMerge/asm/test.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=297627&r1=297626&r2=297627&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Mar 13 10:32:24 2017
@@ -5218,13 +5218,17 @@ Stmt *ASTNodeImporter::VisitGCCAsmStmt(G
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }

Modified: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp?rev=297627&r1=297626&r2=297627&view=diff
==
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp (original)
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp Mon Mar 13 10:32:24 2017
@@ -9,3 +9,13 @@ unsigned char asmFunc(unsigned char a, u
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}

Modified: cfe/trunk/test/ASTMerge/asm/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/asm/test.cpp?rev=297627&r1=297626&r2=297627&view=diff
==
--- cfe/trunk/test/ASTMerge/asm/test.cpp (original)
+++ cfe/trunk/test/ASTMerge/asm/test.cpp Mon Mar 13 10:32:24 2017
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }


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

[PATCH] D30831: [ASTImporter] Import fix of GCCAsmStmts w/ missing symbolic operands

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297627: [ASTImporter] Import fix of GCCAsmStmts w/ missing 
symbolic operands (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D30831?vs=91541&id=91567#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30831

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
  cfe/trunk/test/ASTMerge/asm/test.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
Index: cfe/trunk/test/ASTMerge/asm/test.cpp
===
--- cfe/trunk/test/ASTMerge/asm/test.cpp
+++ cfe/trunk/test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5218,13 +5218,17 @@
   SmallVector Names;
   for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for output operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getOutputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
   for (unsigned I = 0, E = S->getNumInputs(); I != E; I++) {
 IdentifierInfo *ToII = Importer.Import(S->getInputIdentifier(I));
-if (!ToII)
+// ToII is nullptr when no symbolic name is given for input operand
+// see ParseStmtAsm::ParseAsmOperandsOpt
+if (!ToII && S->getInputIdentifier(I))
   return nullptr;
 Names.push_back(ToII);
   }
Index: cfe/trunk/test/ASTMerge/asm/test.cpp
===
--- cfe/trunk/test/ASTMerge/asm/test.cpp
+++ cfe/trunk/test/ASTMerge/asm/test.cpp
@@ -4,4 +4,5 @@
 
 void testAsmImport() {
   asmFunc(12, 42);
+  asmFunc2(42);
 }
Index: cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
===
--- cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
+++ cfe/trunk/test/ASTMerge/asm/Inputs/asm-function.cpp
@@ -9,3 +9,13 @@
   res = bigres;
   return res;
 }
+
+int asmFunc2(int i) {
+  int res;
+  asm ("mov %1, %0 \t\n"
+   "inc %0 "
+  : "=r" (res)
+  : "r" (i)
+  : "cc");
+  return res;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

bkramer: ping


https://reviews.llvm.org/D30720



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

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a project: clang-tools-extra.
Herald added subscribers: JDevlieghere, mgorny.

Add a check to find enums used in `==` or `!=` expressions. Using a switch 
statement leads to more easily maintained code.


Repository:
  rL LLVM

https://reviews.llvm.org/D30896

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
  clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp

Index: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s misc-prefer-switch-for-enums %t 
+
+enum class kind { a, b, c, d };
+
+int foo(kind k)
+{
+  if (k == kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return -1;
+  return 1;
+}
+
+int antifoo(kind k)
+{
+  if (k != kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return 1;
+  return -1;
+}
+
+int bar(int i)
+{
+  if (i == 0)
+return -1;
+  return 1;
+}
+
+int foobar(kind k)
+{
+  switch(k)
+  {
+case kind::a:
+  return -1;
+case kind::b:
+case kind::c:
+case kind::d:
+  return 1;
+  }
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - misc-prefer-switch-for-enums
+
+misc-prefer-switch-for-enums
+
+
+This check finds enum values used in ``==`` and ``!=`` expressions.
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value
+comparison.  Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for
+unhandled cases.
+
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+  
+  int foo(kind k) { 
+return k == kind::a 
+   ? 1 
+   : -1; 
+  }
+
+is more maintainably (but more verbosely) written as:
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+  
+  int foo(kind k) { 
+switch(k) { 
+  case kind::a:
+return 1;
+  case kind::b:
+  case kind::c:
+return -1;
+}
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
+   misc-prefer-switch-for-enums
misc-redundant-expression
misc-sizeof-container
misc-sizeof-expression
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferSwitchForEnumsCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find places where an enumeration value is used in `==` or `!=`. 
+/// Using `switch` leads to more maintainable code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-prefer-switch-for-enums.html
+class PreferSwitchForEnumsCheck : public ClangTidyCheck {
+public:
+  PreferSwitchForEnumsCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsC

r297628 - [CodeCompletion] Format block parameter placeholders in implicit property

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Mar 13 10:43:42 2017
New Revision: 297628

URL: http://llvm.org/viewvc/llvm-project?rev=297628&view=rev
Log:
[CodeCompletion] Format block parameter placeholders in implicit property
setters using the block type information that's obtained from the property

rdar://12604235

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-block-properties.m

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=297628&r1=297627&r2=297628&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Mar 13 10:43:42 2017
@@ -2288,6 +2288,15 @@ static std::string FormatFunctionParamet
   FunctionProtoTypeLoc BlockProto;
   findTypeLocationForBlockDecl(Param->getTypeSourceInfo(), Block, BlockProto,
SuppressBlock);
+  // Try to retrieve the block type information from the property if this is a
+  // parameter in a setter.
+  if (!Block && ObjCMethodParam &&
+  cast(Param->getDeclContext())->isPropertyAccessor()) {
+if (const auto *PD = cast(Param->getDeclContext())
+ ->findPropertyDecl(/*CheckOverrides=*/false))
+  findTypeLocationForBlockDecl(PD->getTypeSourceInfo(), Block, BlockProto,
+   SuppressBlock);
+  }
 
   if (!Block) {
 // We were unable to find a FunctionProtoTypeLoc with parameter names

Modified: cfe/trunk/test/Index/complete-block-properties.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-block-properties.m?rev=297628&r1=297627&r2=297628&view=diff
==
--- cfe/trunk/test/Index/complete-block-properties.m (original)
+++ cfe/trunk/test/Index/complete-block-properties.m Mon Mar 13 10:43:42 2017
@@ -68,8 +68,8 @@ void noQualifierParens(NoQualifierParens
 // RUN: c-index-test -code-completion-at=%s:65:6 %s | FileCheck 
-check-prefix=CHECK-CC2 %s
 //CHECK-CC2: ObjCInstanceMethodDecl:{ResultType void (^)(void)}{TypedText 
blockProperty} (35)
 //CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType BarBlock}{TypedText 
blockProperty2} (35)
-//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty2:}{Placeholder BarBlock blockProperty2} (35)
-//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty:}{Placeholder void (^)(void)blockProperty} (35)
+//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty2:}{Placeholder ^int(int *)blockProperty2} (35)
+//CHECK-CC2-NEXT: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlockProperty:}{Placeholder ^(void)blockProperty} (35)
 
 @interface ClassProperties
 
@@ -86,3 +86,9 @@ void classBlockProperties() {
 //CHECK-CC3: ObjCPropertyDecl:{ResultType void}{TypedText explicit}{LeftParen 
(}{RightParen )} (35)
 //CHECK-CC3-NEXT: ObjCPropertyDecl:{ResultType void (^)()}{TypedText 
explicit}{Equal  = }{Placeholder ^(void)} (38)
 //CHECK-CC3-NEXT: ObjCPropertyDecl:{ResultType void}{TypedText 
explicitReadonly}{LeftParen (}{RightParen )} (35)
+
+void implicitSetterBlockPlaceholder(Test* test) {
+  [test setBlock: ^{}];
+}
+// RUN: c-index-test -code-completion-at=%s:91:9 %s | FileCheck 
-check-prefix=CHECK-CC4 %s
+// CHECK-CC4: ObjCInstanceMethodDecl:{ResultType void}{TypedText 
setBlocker:}{Placeholder ^Foo(int x, Foo y, FooBlock foo)blocker} (37)


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

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added a comment.

lg as a prototype.


https://reviews.llvm.org/D30720



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

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I like that check. But I think it could take another feature.
In my opinion it would be valueable to check if enums are compared against ints 
as well.

For example

  enum Kind k = Kind::a;
  if (k == 3) { /* something */ }

This usecase is not specially considered here, but i think that check would be 
the right place. Maybe it is already covered by your matcher, since you check 
for everything that compares against enums.
If yes, additional diagnostics would be nice.

This check should be added to the ReleaseNotes as well.




Comment at: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp:21
+void PreferSwitchForEnumsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: Add matchers.
+  Finder->addMatcher(

remove `FIXME`



Comment at: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp:27
+ hasLHS(declRefExpr(hasDeclaration(enumConstantDecl()))
+  .bind("x"),
+  this);

maybe a more telling name than `x` would be better?



Comment at: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp:3
+
+enum class kind { a, b, c, d };
+

maybe another test for non enum class values would be nice.

```
enum another_kind {e, f, g};
```




Repository:
  rL LLVM

https://reviews.llvm.org/D30896



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

[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

In https://reviews.llvm.org/D30158#699132, @madsravn wrote:

> In https://reviews.llvm.org/D30158#698871, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D30158#696534, @madsravn wrote:
> >
> > > Any updates on this?
> >
> >
> > Have you run it over the test suite on the revision before random_shuffle 
> > was removed from libc++?
>
>
> I can't find any more tests for random_shuffle. I have looked in the SVN log 
> for from december 2014 until now. It works on the three tests currently in 
> trunk.


Try just before r294328.


https://reviews.llvm.org/D30158



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

[clang-tools-extra] r297630 - [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Mar 13 10:55:59 2017
New Revision: 297630

URL: http://llvm.org/viewvc/llvm-project?rev=297630&view=rev
Log:
[include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

Summary:
Add fuzzy SymbolIndex, where identifier needn't match exactly.

The purpose for this is global autocomplete in clangd. The query will be a
partial identifier up to the cursor, and the results will be suggestions.

It's in include-fixer because:

  - it handles SymbolInfos, actually SymbolIndex is exactly the right interface
  - it's a good harness for lit testing the fuzzy YAML index
  - (Laziness: we can't unit test clangd until reorganizing with a tool/ dir)

Other questionable choices:

  - FuzzySymbolIndex, which just refines the contract of SymbolIndex. This is
an interface to allow extension to large monorepos (*cough*)
  - an always-true safety check that Identifier == Name is removed from
SymbolIndexManager, as it's not true for fuzzy matching
  - exposing -db=fuzzyYaml from include-fixer is not a very useful feature, and
a non-orthogonal ui (fuzziness vs data source). -db=fixed is similar though.

Reviewers: bkramer

Subscribers: cfe-commits, mgorny

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

Added:
clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.h
clang-tools-extra/trunk/test/include-fixer/yaml_fuzzy.cpp
clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
Modified:
clang-tools-extra/trunk/include-fixer/CMakeLists.txt
clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt

Modified: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/CMakeLists.txt?rev=297630&r1=297629&r2=297630&view=diff
==
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt Mon Mar 13 10:55:59 
2017
@@ -6,6 +6,7 @@ add_clang_library(clangIncludeFixer
   IncludeFixer.cpp
   IncludeFixerContext.cpp
   InMemorySymbolIndex.cpp
+  FuzzySymbolIndex.cpp
   SymbolIndexManager.cpp
   YamlSymbolIndex.cpp
 

Added: clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp?rev=297630&view=auto
==
--- clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp (added)
+++ clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp Mon Mar 13 
10:55:59 2017
@@ -0,0 +1,143 @@
+//===--- FuzzySymbolIndex.cpp - Lookup symbols for autocomplete -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "FuzzySymbolIndex.h"
+#include "llvm/Support/Regex.h"
+
+using clang::find_all_symbols::SymbolAndSignals;
+using llvm::StringRef;
+
+namespace clang {
+namespace include_fixer {
+namespace {
+
+class MemSymbolIndex : public FuzzySymbolIndex {
+public:
+  MemSymbolIndex(std::vector Symbols) {
+for (auto &Symbol : Symbols) {
+  auto Tokens = tokenize(Symbol.Symbol.getName());
+  this->Symbols.emplace_back(
+  StringRef(llvm::join(Tokens.begin(), Tokens.end(), " ")),
+  std::move(Symbol));
+}
+  }
+
+  std::vector search(StringRef Query) override {
+auto Tokens = tokenize(Query);
+llvm::Regex Pattern("^" + queryRegexp(Tokens));
+std::vector Results;
+for (const Entry &E : Symbols)
+  if (Pattern.match(E.first))
+Results.push_back(E.second);
+return Results;
+  }
+
+private:
+  using Entry = std::pair, SymbolAndSignals>;
+  std::vector Symbols;
+};
+
+// Helpers for tokenize state machine.
+enum TokenizeState {
+  EMPTY,  // No pending characters.
+  ONE_BIG,// Read one uppercase letter, could be WORD or Word.
+  BIG_WORD,   // Reading an uppercase WORD.
+  SMALL_WORD, // Reading a lowercase word.
+  NUMBER  // Reading a number.
+};
+
+enum CharType { UPPER, LOWER, DIGIT, MISC };
+CharType classify(char c) {
+  if (isupper(c))
+return UPPER;
+  if (islower(c))
+return LOWER;
+  if (isdigit(c))
+return DIGIT;
+  return MISC;
+}
+
+} // namespace
+
+std::vector FuzzySymbolIndex::tokenize(StringRef Text) {
+  std::vector Result;
+  // State describes the treatment of text from Start to I.
+  // Once text is Flush()ed into Result, we're done with it and advance Start.
+  Token

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297630: [include-fixer] Add fuzzy SymbolIndex, where 
identifier needn't match exactly. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D30720?vs=91042&id=91570#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30720

Files:
  clang-tools-extra/trunk/include-fixer/CMakeLists.txt
  clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.cpp
  clang-tools-extra/trunk/include-fixer/FuzzySymbolIndex.h
  clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
  clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
  clang-tools-extra/trunk/test/include-fixer/yaml_fuzzy.cpp
  clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
  clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp

Index: clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
===
--- clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
+++ clang-tools-extra/trunk/unittests/include-fixer/CMakeLists.txt
@@ -13,6 +13,7 @@
 
 add_extra_unittest(IncludeFixerTests
   IncludeFixerTest.cpp
+  FuzzySymbolIndexTests.cpp
   )
 
 target_link_libraries(IncludeFixerTests
Index: clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/FuzzySymbolIndexTests.cpp
@@ -0,0 +1,61 @@
+//===-- FuzzySymbolIndexTests.cpp - Fuzzy symbol index unit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FuzzySymbolIndex.h"
+#include "gmock/gmock.h"
+#include "llvm/Support/Regex.h"
+#include "gtest/gtest.h"
+
+using testing::ElementsAre;
+using testing::Not;
+
+namespace clang {
+namespace include_fixer {
+namespace {
+
+TEST(FuzzySymbolIndexTest, Tokenize) {
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("URLHandlerCallback"),
+  ElementsAre("url", "handler", "callback"));
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("snake_case11"),
+  ElementsAre("snake", "case", "11"));
+  EXPECT_THAT(FuzzySymbolIndex::tokenize("__$42!!BOB\nbob"),
+  ElementsAre("42", "bob", "bob"));
+}
+
+MATCHER_P(MatchesSymbol, Identifier, "") {
+  llvm::Regex Pattern("^" + arg);
+  std::string err;
+  if (!Pattern.isValid(err)) {
+*result_listener << "invalid regex: " << err;
+return false;
+  }
+  auto Tokens = FuzzySymbolIndex::tokenize(Identifier);
+  std::string Target = llvm::join(Tokens.begin(), Tokens.end(), " ");
+  *result_listener << "matching against '" << Target << "'";
+  return llvm::Regex("^" + arg).match(Target);
+}
+
+TEST(FuzzySymbolIndexTest, QueryRegexp) {
+  auto QueryRegexp = [](const std::string &query) {
+return FuzzySymbolIndex::queryRegexp(FuzzySymbolIndex::tokenize(query));
+  };
+  EXPECT_THAT(QueryRegexp("uhc"), MatchesSymbol("URLHandlerCallback"));
+  EXPECT_THAT(QueryRegexp("urhaca"), MatchesSymbol("URLHandlerCallback"));
+  EXPECT_THAT(QueryRegexp("uhcb"), Not(MatchesSymbol("URLHandlerCallback")))
+  << "Non-prefix";
+  EXPECT_THAT(QueryRegexp("uc"), Not(MatchesSymbol("URLHandlerCallback")))
+  << "Skip token";
+
+  EXPECT_THAT(QueryRegexp("uptr"), MatchesSymbol("unique_ptr"));
+  EXPECT_THAT(QueryRegexp("UniP"), MatchesSymbol("unique_ptr"));
+}
+
+} // namespace
+} // namespace include_fixer
+} // namespace clang
Index: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
===
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt
@@ -6,6 +6,7 @@
   IncludeFixer.cpp
   IncludeFixerContext.cpp
   InMemorySymbolIndex.cpp
+  FuzzySymbolIndex.cpp
   SymbolIndexManager.cpp
   YamlSymbolIndex.cpp
 
Index: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "FuzzySymbolIndex.h"
 #include "InMemorySymbolIndex.h"
 #include "IncludeFixer.h"
 #include "IncludeFixerContext.h"
@@ -83,14 +84,16 @@
 cl::OptionCategory IncludeFixerCategory("Tool options");
 
 enum DatabaseFormatTy {
-  fixed, ///< Hard-coded mapping.
-  yaml,  ///< Yaml database created by find-all-symbols.
+  fixed, ///< Hard-coded mapping.
+  ya

[PATCH] D30898: Add new -fverbose-asm that enables source interleaving

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.

This is the clang side of the RFC in 
http://lists.llvm.org/pipermail/cfe-dev/2017-February/052549.html

Note that in contrast to the original suggestion `-fsource-asm` here we use the 
preferred `-fverbose-asm`. Basically explicitly saying `-fverbose-asm` in the 
command line enables a minimum amount of debugging, so in AsmPrinter we can use 
it to print the source code.

This patch introduces a `-masm-source` flag for cc1 that maps to the AsmSource 
value in the llvm code generation.


https://reviews.llvm.org/D30898

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -560,6 +560,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
+  Opts.AsmSource = getLastArgIntValue(Args, OPT_masm_source, 0, Diags);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2748,6 +2748,20 @@
 CmdArgs.push_back("-split-dwarf=Enable");
   }
 
+  // If -fverbose-asm explicitly appears enable at least DebugLineTablesOnly
+  // but remember that no debug info was requested, to avoid cluttering
+  // assembler output with debug directives.
+  if (Args.hasArg(options::OPT_fverbose_asm)) {
+CmdArgs.push_back("-masm-source");
+if (DebugInfoKind == codegenoptions::NoDebugInfo) {
+  // FIXME: Check whether LimitedDebugInfo will give us line information,
+  // otherwise we should be overriding it as well.
+  DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+  CmdArgs.push_back("1");
+} else
+  CmdArgs.push_back("2");
+  }
+
   // After we've dealt with all combinations of things that could
   // make DebugInfoKind be other than None or DebugLineTablesOnly,
   // figure out if we need to "upgrade" it to standalone debug info.
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -604,6 +604,7 @@
   Options.MCOptions.MCPIECopyRelocations = CodeGenOpts.PIECopyRelocations;
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.AsmSource = CodeGenOpts.AsmSource;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto &Entry : HSOpts.UserEntries)
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -32,6 +32,7 @@
 CODEGENOPT(CompressDebugSections, 1, 0) ///< -Wa,-compress-debug-sections
 CODEGENOPT(RelaxELFRelocations, 1, 0) ///< -Wa,--mrelax-relocations
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(AsmSource , 2, 0) ///< -fverbose-asm.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.
 CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit __attribute__((malloc)) 
operator new
 CODEGENOPT(Autolink  , 1, 1) ///< -fno-autolink
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -225,6 +225,8 @@
   HelpText<"Turn off struct-path aware Type Based Alias Analysis">;
 def masm_verbose : Flag<["-"], "masm-verbose">,
   HelpText<"Generate verbose assembly output">;
+def masm_source : Separate<["-"], "masm-source">,
+  HelpText<"Annotate assembly output with source code lines.">;
 def mcode_model : Separate<["-"], "mcode-model">,
   HelpText<"The code model to use">;
 def mdebug_pass : Separate<["-"], "mdebug-pass">,


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -560,6 +560,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = Args.hasArg(OPT_masm_verbose);
+  Opts.AsmSource = getLastArgIntValue(Args, OPT_masm_source, 0, Diags);
   Opt

[PATCH] D30766: Add support for attribute "enum_extensibility"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 91577.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Address Alex's review comments.


https://reviews.llvm.org/D30766

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaStmt.cpp
  test/Sema/enum-attr.c
  test/SemaCXX/attr-flag-enum-reject.cpp
  test/SemaCXX/enum-attr.cpp

Index: test/SemaCXX/enum-attr.cpp
===
--- /dev/null
+++ test/SemaCXX/enum-attr.cpp
@@ -0,0 +1,108 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default -std=c++11 %s
+
+enum Enum {
+  A0 = 1, A1 = 10
+};
+
+enum __attribute__((enum_extensibility(closed))) EnumClosed {
+  B0 = 1, B1 = 10
+};
+
+enum [[clang::enum_extensibility(open)]] EnumOpen {
+  C0 = 1, C1 = 10
+};
+
+enum __attribute__((flag_enum)) EnumFlag {
+  D0 = 1, D1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed {
+  E0 = 1, E1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen {
+  F0 = 1, F1 = 8
+};
+
+void test() {
+  enum Enum t0;
+
+  switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}}
+  case A0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t0) {
+  case A0: break;
+  case A1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumClosed t1;
+
+  switch (t1) { // expected-warning{{enumeration value 'B1' not handled in switch}}
+  case B0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t1) {
+  case B0: break;
+  case B1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumOpen t2;
+
+  switch (t2) { // expected-warning{{enumeration value 'C1' not handled in switch}}
+  case C0: break;
+  case 16: break;
+  }
+
+  switch (t2) {
+  case C0: break;
+  case C1: break;
+  default: break;
+  }
+
+  enum EnumFlag t3;
+
+  switch (t3) { // expected-warning{{enumeration value 'D1' not handled in switch}}
+  case D0: break;
+  case 9: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t3) {
+  case D0: break;
+  case D1: break;
+  default: break;
+  }
+
+  enum EnumFlagClosed t4;
+
+  switch (t4) { // expected-warning{{enumeration value 'E1' not handled in switch}}
+  case E0: break;
+  case 9: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t4) {
+  case E0: break;
+  case E1: break;
+  default: break;
+  }
+
+  enum EnumFlagOpen t5;
+
+  switch (t5) { // expected-warning{{enumeration value 'F1' not handled in switch}}
+  case F0: break;
+  case 9: break;
+  case 16: break;
+  }
+
+  switch (t5) {
+  case F0: break;
+  case F1: break;
+  default: break;
+  }
+}
Index: test/SemaCXX/attr-flag-enum-reject.cpp
===
--- test/SemaCXX/attr-flag-enum-reject.cpp
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -x c++ -Wassign-enum %s
-
-enum __attribute__((flag_enum)) flag { // expected-warning {{ignored}}
-};
Index: test/Sema/enum-attr.c
===
--- /dev/null
+++ test/Sema/enum-attr.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wassign-enum -Wswitch-enum -Wcovered-switch-default %s
+
+enum Enum {
+  A0 = 1, A1 = 10
+};
+
+enum __attribute__((enum_extensibility(closed))) EnumClosed {
+  B0 = 1, B1 = 10
+};
+
+enum __attribute__((enum_extensibility(open))) EnumOpen {
+  C0 = 1, C1 = 10
+};
+
+enum __attribute__((flag_enum)) EnumFlag {
+  D0 = 1, D1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(closed))) EnumFlagClosed {
+  E0 = 1, E1 = 8
+};
+
+enum __attribute__((flag_enum,enum_extensibility(open))) EnumFlagOpen {
+  F0 = 1, F1 = 8
+};
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // expected-error{{'enum_extensibility' attribute argument not supported: 'arg1'}}
+  G
+};
+
+void test() {
+  enum Enum t0 = 100; // expected-warning{{integer constant not in range of enumerated type}}
+  t0 = 1;
+
+  switch (t0) { // expected-warning{{enumeration value 'A1' not handled in switch}}
+  case A0: break;
+  case 16: break; // expected-warning{{case value not in enumerated type}}
+  }
+
+  switch (t0) {
+  case A0: break;
+  case A1: break;
+  default: break; // expected-warning{{default label in switch which covers all enumeration}}
+  }
+
+  enum EnumClosed t1 = 100; // expected-warning{{integer constant not in range of enumerated type}}
+  t1 = 1;
+
+  switch (t1) { // expected-warning{{enumeration value 'B1' not handled 

[PATCH] D30766: Add support for attribute "enum_extensibility"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/Sema/enum-attr.c:27
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // 
expected-warning{{'enum_extensibility' attribute argument not supported: 
'arg1'}}
+  G

arphaman wrote:
> Should this be an error instead?
Yes, I agree.


https://reviews.llvm.org/D30766



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

[PATCH] D30816: [OpenCL] Added implicit conversion rank for overloading functions with vector data type in OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/overload-scalar-widening.cl:4
+
+typedef short short4 __attribute__((ext_vector_type(4)));
+

I am thinking could this be a CodeGen test instead and we could check that the 
right overload is selected based on mangled name?

I think in this case it would be good to unify with 
test/SemaOpenCL/overload_addrspace_resolution.cl which has similar purpose. 
Also I think CodeGenOpenCL would be a better place for it. :) 


https://reviews.llvm.org/D30816



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Would you be able to update ScalarExprEmitter::VisitAsTypeExpr implementation 
accordingly to make things consistent?


https://reviews.llvm.org/D30810



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

Re: Patch for Bug 30413, including test case

[Akira Hatanaka via cfe-commits]

Do you need someone to commit this patch for you?

> On Mar 10, 2017, at 6:44 AM, Lobron, David  wrote:
> 
> Hi Akira,
> 
> Thank you very much!  Please let me know if I need to take any further steps 
> beyond this email to cfe-commits in order for the patch and the unit test to 
> be committed.
> 
> Thanks,
> 
> David
> 
>> On Mar 9, 2017, at 4:46 PM, Akira Hatanaka  wrote:
>> 
>> Hi David,
>> 
>> The patch looks good to me.
>> 
>>> On Mar 9, 2017, at 1:01 PM, Lobron, David  wrote:
>>> 
>>> Hi Akira,
>>> 
 My concern is that the patch changes the encoding of @encode(id) 
 on Darwin, which I think isn’t what you are trying to fix. If you compile 
 the following code with command “clang -cc1 -triple x86_64-apple-macosx”, 
 the type encoding changes after applying the patch.
 
 const char *foo() {
 return @encode(id);
 }
 
 It seems like you can fix your problem without affecting Darwin by passing 
 an extra argument to getObjCEncodingForType, just like 
 CGObjCCommonMac::GetMethodVarType does.
>>> 
>>> Ah, thanks- I understand now.  Yes, this change seems a lot safer, and I 
>>> verified that it passes my test.  I've attached my new patch file, and I've 
>>> also attached the test again.  Please let me know if this works for you or 
>>> if you think it needs any additional work.
>>> 
>>> --David
>>> 
>>> 
>> 
> 

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

Re: Patch for Bug 30413, including test case

[Lobron, David via cfe-commits]

Yes, please, if you don't mind!  I'd like to commit both the path and the unit 
test, if possible.

Thanks,

David

> On Mar 13, 2017, at 12:47 PM, Akira Hatanaka  wrote:
> 
> Do you need someone to commit this patch for you?
> 
>> On Mar 10, 2017, at 6:44 AM, Lobron, David  wrote:
>> 
>> Hi Akira,
>> 
>> Thank you very much!  Please let me know if I need to take any further steps 
>> beyond this email to cfe-commits in order for the patch and the unit test to 
>> be committed.
>> 
>> Thanks,
>> 
>> David
>> 
>>> On Mar 9, 2017, at 4:46 PM, Akira Hatanaka  wrote:
>>> 
>>> Hi David,
>>> 
>>> The patch looks good to me.
>>> 
 On Mar 9, 2017, at 1:01 PM, Lobron, David  wrote:
 
 Hi Akira,
 
> My concern is that the patch changes the encoding of 
> @encode(id) on Darwin, which I think isn’t what you are trying 
> to fix. If you compile the following code with command “clang -cc1 
> -triple x86_64-apple-macosx”, the type encoding changes after applying 
> the patch.
> 
> const char *foo() {
> return @encode(id);
> }
> 
> It seems like you can fix your problem without affecting Darwin by 
> passing an extra argument to getObjCEncodingForType, just like 
> CGObjCCommonMac::GetMethodVarType does.
 
 Ah, thanks- I understand now.  Yes, this change seems a lot safer, and I 
 verified that it passes my test.  I've attached my new patch file, and 
 I've also attached the test again.  Please let me know if this works for 
 you or if you think it needs any additional work.
 
 --David
 
 
>>> 
>> 
> 

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

r297642 - [X86] Add checking of the scale argument to scatter/gather builtins

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Mar 13 12:16:50 2017
New Revision: 297642

URL: http://llvm.org/viewvc/llvm-project?rev=297642&view=rev
Log:
[X86] Add checking of the scale argument to scatter/gather builtins

The only valid values for scale immediate of scatter/gather builtins are 1, 2, 
4, or 8. This patch enforces this in the frontend otherwise we generate invalid 
instruction encodings in the backend.

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



Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/builtins-x86.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=297642&r1=297641&r2=297642&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Mar 13 12:16:50 
2017
@@ -8005,6 +8005,8 @@ def err_x86_builtin_32_bit_tgt : Error<
   "this builtin is only available on x86-64 targets">;
 def err_x86_builtin_invalid_rounding : Error<
   "invalid rounding argument">;
+def err_x86_builtin_invalid_scale : Error<
+  "scale argument must be 1, 2, 4, or 8">;
 
 def err_builtin_longjmp_unsupported : Error<
   "__builtin_longjmp is not supported for the current target">;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=297642&r1=297641&r2=297642&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Mar 13 12:16:50 2017
@@ -9993,6 +9993,7 @@ private:
   bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckSystemZBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall);
+  bool CheckX86BuiltinGatherScatterScale(unsigned BuiltinID, CallExpr 
*TheCall);
   bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=297642&r1=297641&r2=297642&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Mar 13 12:16:50 2017
@@ -1986,6 +1986,109 @@ bool Sema::CheckX86BuiltinRoundingOrSAE(
 << Arg->getSourceRange();
 }
 
+// Check if the gather/scatter scale is legal.
+bool Sema::CheckX86BuiltinGatherScatterScale(unsigned BuiltinID,
+ CallExpr *TheCall) {
+  unsigned ArgNum = 0;
+  switch (BuiltinID) {
+  default:
+return false;
+  case X86::BI__builtin_ia32_gatherpfdpd:
+  case X86::BI__builtin_ia32_gatherpfdps:
+  case X86::BI__builtin_ia32_gatherpfqpd:
+  case X86::BI__builtin_ia32_gatherpfqps:
+  case X86::BI__builtin_ia32_scatterpfdpd:
+  case X86::BI__builtin_ia32_scatterpfdps:
+  case X86::BI__builtin_ia32_scatterpfqpd:
+  case X86::BI__builtin_ia32_scatterpfqps:
+ArgNum = 3;
+break;
+  case X86::BI__builtin_ia32_gatherd_pd:
+  case X86::BI__builtin_ia32_gatherd_pd256:
+  case X86::BI__builtin_ia32_gatherq_pd:
+  case X86::BI__builtin_ia32_gatherq_pd256:
+  case X86::BI__builtin_ia32_gatherd_ps:
+  case X86::BI__builtin_ia32_gatherd_ps256:
+  case X86::BI__builtin_ia32_gatherq_ps:
+  case X86::BI__builtin_ia32_gatherq_ps256:
+  case X86::BI__builtin_ia32_gatherd_q:
+  case X86::BI__builtin_ia32_gatherd_q256:
+  case X86::BI__builtin_ia32_gatherq_q:
+  case X86::BI__builtin_ia32_gatherq_q256:
+  case X86::BI__builtin_ia32_gatherd_d:
+  case X86::BI__builtin_ia32_gatherd_d256:
+  case X86::BI__builtin_ia32_gatherq_d:
+  case X86::BI__builtin_ia32_gatherq_d256:
+  case X86::BI__builtin_ia32_gather3div2df:
+  case X86::BI__builtin_ia32_gather3div2di:
+  case X86::BI__builtin_ia32_gather3div4df:
+  case X86::BI__builtin_ia32_gather3div4di:
+  case X86::BI__builtin_ia32_gather3div4sf:
+  case X86::BI__builtin_ia32_gather3div4si:
+  case X86::BI__builtin_ia32_gather3div8sf:
+  case X86::BI__builtin_ia32_gather3div8si:
+  case X86::BI__builtin_ia32_gather3siv2df:
+  case X86::BI__builtin_ia32_gather3siv2di:
+  case X86::BI__builtin_ia32_gather3siv4df:
+  case X86::BI__builtin_ia32_gather3siv4di:
+  case X86::BI__builtin_ia32_gather3siv4sf:
+  case X86::BI__builtin_ia32_gather3siv4si:
+  case X86::BI__builtin_ia32_gather3siv8sf:
+  case X86::BI__builtin_ia32_gather3siv8si:
+  case X86::BI__builtin_ia32_gathersiv8df:
+  case X86::BI__builtin_ia32_gathersiv16sf:
+  case X86::BI__builtin_ia32_gatherdiv8df:
+  case X86::BI__bu

[PATCH] D30875: [X86] Add checking of the scale argument to scatter/gather builtins

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL297642: [X86] Add checking of the scale argument to 
scatter/gather builtins (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D30875?vs=91508&id=91586#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30875

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/builtins-x86.c

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -9993,6 +9993,7 @@
   bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckSystemZBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinRoundingOrSAE(unsigned BuiltinID, CallExpr *TheCall);
+  bool CheckX86BuiltinGatherScatterScale(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
 
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8005,6 +8005,8 @@
   "this builtin is only available on x86-64 targets">;
 def err_x86_builtin_invalid_rounding : Error<
   "invalid rounding argument">;
+def err_x86_builtin_invalid_scale : Error<
+  "scale argument must be 1, 2, 4, or 8">;
 
 def err_builtin_longjmp_unsupported : Error<
   "__builtin_longjmp is not supported for the current target">;
Index: cfe/trunk/test/Sema/builtins-x86.c
===
--- cfe/trunk/test/Sema/builtins-x86.c
+++ cfe/trunk/test/Sema/builtins-x86.c
@@ -4,6 +4,7 @@
 typedef float __m128 __attribute__((__vector_size__(16)));
 typedef double __m128d __attribute__((__vector_size__(16)));
 
+typedef long long __m512i __attribute__((__vector_size__(64)));
 typedef float __m512 __attribute__((__vector_size__(64)));
 typedef double __m512d __attribute__((__vector_size__(64)));
 
@@ -69,3 +70,16 @@
 __mmask16 test__builtin_ia32_cmpps512_mask_rounding(__m512 __a, __m512 __b, __mmask16 __u) {
   __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 0); // expected-error {{invalid rounding argument}}
 }
+
+__m128i test_mm_mask_i32gather_epi32(__m128i a, int const *b, __m128i c, __m128i mask) {
+  return __builtin_ia32_gatherd_d(a, b, c, mask, 5); // expected-error {{scale argument must be 1, 2, 4, or 8}}
+}
+
+__m512i _mm512_mask_prefetch_i32gather_ps(__m512i index, __mmask16 mask, int const *addr) {
+  return __builtin_ia32_gatherpfdps(mask, index, addr, 5, 1); // expected-error {{scale argument must be 1, 2, 4, or 8}}
+}
+
+__m512 _mm512_mask_prefetch_i32gather_ps_2(__m512i index, __mmask16 mask, int const *addr) {
+  return __builtin_ia32_gatherpfdps(mask, index, addr, 1, 3); // expected-error {{argument should be a value from 1 to 2}}
+}
+
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -1986,6 +1986,109 @@
 << Arg->getSourceRange();
 }
 
+// Check if the gather/scatter scale is legal.
+bool Sema::CheckX86BuiltinGatherScatterScale(unsigned BuiltinID,
+ CallExpr *TheCall) {
+  unsigned ArgNum = 0;
+  switch (BuiltinID) {
+  default:
+return false;
+  case X86::BI__builtin_ia32_gatherpfdpd:
+  case X86::BI__builtin_ia32_gatherpfdps:
+  case X86::BI__builtin_ia32_gatherpfqpd:
+  case X86::BI__builtin_ia32_gatherpfqps:
+  case X86::BI__builtin_ia32_scatterpfdpd:
+  case X86::BI__builtin_ia32_scatterpfdps:
+  case X86::BI__builtin_ia32_scatterpfqpd:
+  case X86::BI__builtin_ia32_scatterpfqps:
+ArgNum = 3;
+break;
+  case X86::BI__builtin_ia32_gatherd_pd:
+  case X86::BI__builtin_ia32_gatherd_pd256:
+  case X86::BI__builtin_ia32_gatherq_pd:
+  case X86::BI__builtin_ia32_gatherq_pd256:
+  case X86::BI__builtin_ia32_gatherd_ps:
+  case X86::BI__builtin_ia32_gatherd_ps256:
+  case X86::BI__builtin_ia32_gatherq_ps:
+  case X86::BI__builtin_ia32_gatherq_ps256:
+  case X86::BI__builtin_ia32_gatherd_q:
+  case X86::BI__builtin_ia32_gatherd_q256:
+  case X86::BI__builtin_ia32_gatherq_q:
+  case X86::BI__builtin_ia32_gatherq_q256:
+  case X86::BI__builtin_ia32_gatherd_d:
+  case X86::BI__builtin_ia32_gatherd_d256:
+  case X86::BI__builtin_ia32_gatherq_d:
+  case X86::BI__builtin_ia32_gatherq_d256:
+  case X86::BI__builtin_ia32_gather3div2df:
+  case X86::BI__builtin_ia32_gather3div2di:
+  case X86::BI__builtin_ia32_gather3div4df:
+  case X86::BI__builtin_ia32_gather3div4di:
+  case X86::BI__builtin_ia32_gather3div4sf:
+  case X86::BI__b

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe updated this revision to Diff 91588.
jbcoe added a comment.

Minor edits in response to review comments. A few questions outstanding.


https://reviews.llvm.org/D30896

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
  clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
  clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp

Index: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s misc-prefer-switch-for-enums %t 
+
+enum class kind { a, b, c, d };
+
+int foo(kind k)
+{
+  if (k == kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return -1;
+  return 1;
+}
+
+int antifoo(kind k)
+{
+  if (k != kind::a)
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+return 1;
+  return -1;
+}
+
+int bar(int i)
+{
+  if (i == 0)
+return -1;
+  return 1;
+}
+
+int foobar(kind k)
+{
+  switch(k)
+  {
+case kind::a:
+  return -1;
+case kind::b:
+case kind::c:
+case kind::d:
+  return 1;
+  }
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - misc-prefer-switch-for-enums
+
+misc-prefer-switch-for-enums
+
+
+This check finds enum values used in ``==`` and ``!=`` expressions.
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value
+comparison.  Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for
+unhandled cases.
+
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+
+  int foo(kind k) {
+return k == kind::a
+   ? 1
+   : -1;
+  }
+
+is more maintainably (but more verbosely) written as:
+
+.. code-block:: c++
+
+  enum class kind { a, b, c};
+
+  int foo(kind k) {
+switch(k) {
+  case kind::a:
+return 1;
+  case kind::b:
+  case kind::c:
+return -1;
+}
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
+   misc-prefer-switch-for-enums
misc-redundant-expression
misc-sizeof-container
misc-sizeof-expression
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -90,6 +90,12 @@
 - Support clang-formatting of the code around applied fixes (``-format-style``
   command-line option).
 
+- New `misc-prefer-switch-for-enums
+  `_ check
+
+  Finds uses of enumeration values in equality and inequality expressions where a switch would be preferred.
+
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferSwitchForEnumsCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find places where an enumeration value is used in `==` or `!=`. 
+/// Using `switch` leads to more maintainable code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-prefer-switch-for-enums.h

[PATCH] D30743: enable -save-temps with -finclude-defult-header

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Driver/Tools.cpp:5288
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {

It would be nice to add a comment around here explaining this case.



Comment at: lib/Driver/Tools.cpp:5290
+  !isa(JA)) {
+//Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {

Remove the commented code, please!


https://reviews.llvm.org/D30743



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

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Jonathan B Coe via Phabricator via cfe-commits]

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

Handling

  enum Kind k = Kind::a;
  if (k == 3) { /* something */ }

is intentionally out of scope for now as the author is doing something that I 
can't trivially replace with a switch.




Comment at: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp:3
+
+enum class kind { a, b, c, d };
+

JonasToth wrote:
> maybe another test for non enum class values would be nice.
> 
> ```
> enum another_kind {e, f, g};
> ```
> 
> 
I don't need to write code to specifically deal with enum classes. Perhaps 
changing the test to just test enums is simpler. I'm not sure what the 
LLVM/Clang approach normally is.


https://reviews.llvm.org/D30896



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

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8263
+def err_atomic_init_addressspace : Error<
+  "initialization of atomic variables is restricted to variables in global 
address space">;
 def err_atomic_init_constant : Error<

bader wrote:
> bader wrote:
> > Anastasia wrote:
> > > echuraev wrote:
> > > > Anastasia wrote:
> > > > > Could we combine this error diag with the one below? I guess they are 
> > > > > semantically very similar apart from one is about initialization and 
> > > > > another is about assignment?
> > > > I'm not sure that it is a good idea to combine these errors. For 
> > > > example, if developer had declared a variable non-constant and not in 
> > > > global address space he would have got the same message for both 
> > > > errors. And it can be difficult to determine what the exact problem is. 
> > > > He can fix one of the problems but he will still get the same error.
> > > Well, I don't actually see that we check for constant anywhere so it's 
> > > also OK if you want to drop this bit. Although I think the original 
> > > intension of this message as I understood was to provide the most 
> > > complete hint.
> > > 
> > > My concern is that these two errors seem to be reporting nearly the same 
> > > issue and ideally we would like to keep diagnostic list as small as 
> > > possible. This also makes the file more concise and messages more 
> > > consistent.
> > I suggest adding a test case with non-constant initialization case to 
> > validate that existing checks cover this case for atomic types already.
> > If so, we can adjust existing diagnostic message to cover both cases: 
> > initialization and assignment expression.
> I don't think it's quite true.
> There are two requirements here that must be met the same time. Atomic 
> variables *declared in the global address space* can be initialized only with 
> "compile time constant'.
> If understand the spec correctly this code is also valid:
> 
>   kernel void foo() {
> static global atomic_int a = 42; // although it's not clear if we must 
> use ATOMIC_VAR_INIT here.
> ...
>   }
Precisely, but I think checking for compile time constant should be inherited 
from the general C implementation? I don't think we do anything extra for it.  
Regarding the macro I am not sure we can suitably diagnose it anyways...



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8268
+// Atomics
+def err_atomic_init: Error<
+  "atomic variable can only be assigned to a compile time constant or to 
variables in global adress space">;

also we should add opencl there:

err_opencl_...


https://reviews.llvm.org/D30643



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

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Florian Gross via Phabricator via cfe-commits]

fgross added a comment.

No commit access, could someone please take care of this? Thanks!


https://reviews.llvm.org/D30610



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

r297649 - [Linker] Provide callback for internalization

[Jonas Devlieghere via cfe-commits]

Author: jdevlieghere
Date: Mon Mar 13 13:08:11 2017
New Revision: 297649

URL: http://llvm.org/viewvc/llvm-project?rev=297649&view=rev
Log:
[Linker] Provide callback for internalization

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

Modified:
cfe/trunk/include/clang/CodeGen/CodeGenAction.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/CodeGen/CodeGenAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenAction.h?rev=297649&r1=297648&r2=297649&view=diff
==
--- cfe/trunk/include/clang/CodeGen/CodeGenAction.h (original)
+++ cfe/trunk/include/clang/CodeGen/CodeGenAction.h Mon Mar 13 13:08:11 2017
@@ -36,6 +36,9 @@ private:
 /// function ourselves.
 bool PropagateAttrs;
 
+/// If true, we use LLVM module internalizer.
+bool Internalize;
+
 /// Bitwise combination of llvm::LinkerFlags used when we link the module.
 unsigned LinkFlags;
   };

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.h?rev=297649&r1=297648&r2=297649&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Mon Mar 13 13:08:11 2017
@@ -137,6 +137,8 @@ public:
 /// our CodeGenOptions, much as we set attrs on functions that we generate
 /// ourselves.
 bool PropagateAttrs = false;
+/// If true, we use LLVM module internalizer.
+bool Internalize = false;
 /// Bitwise combination of llvm::Linker::Flags, passed to the LLVM linker.
 unsigned LinkFlags = 0;
   };

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=297649&r1=297648&r2=297649&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Mar 13 13:08:11 2017
@@ -28,6 +28,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IRReader/IRReader.h"
@@ -38,6 +39,8 @@
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "llvm/Transforms/IPO/Internalize.h"
+
 #include 
 using namespace clang;
 using namespace llvm;
@@ -168,8 +171,22 @@ namespace clang {
 Gen->CGM().AddDefaultFnAttrs(F);
 
 CurLinkModule = LM.Module.get();
-if (Linker::linkModules(*getModule(), std::move(LM.Module),
-LM.LinkFlags))
+
+bool Err;
+if (LM.Internalize) {
+  Err = Linker::linkModules(
+  *getModule(), std::move(LM.Module), LM.LinkFlags,
+  [](llvm::Module &M, const llvm::StringSet<> &GVS) {
+internalizeModule(M, [&M, &GVS](const llvm::GlobalValue &GV) {
+  return !GV.hasName() || (GVS.count(GV.getName()) == 0);
+});
+  });
+} else {
+  Err = Linker::linkModules(*getModule(), std::move(LM.Module),
+LM.LinkFlags);
+}
+
+if (Err)
   return true;
   }
   return false; // success
@@ -319,7 +336,7 @@ namespace clang {
 void OptimizationFailureHandler(
 const llvm::DiagnosticInfoOptimizationFailure &D);
   };
-  
+
   void BackendConsumer::anchor() {}
 }
 
@@ -388,7 +405,7 @@ void BackendConsumer::InlineAsmDiagHandl
   // code.
   if (LocCookie.isValid()) {
 Diags.Report(LocCookie, DiagID).AddString(Message);
-
+
 if (D.getLoc().isValid()) {
   DiagnosticBuilder B = Diags.Report(Loc, diag::note_fe_inline_asm_here);
   // Convert the SMDiagnostic ranges into SourceRange and attach them
@@ -401,7 +418,7 @@ void BackendConsumer::InlineAsmDiagHandl
 }
 return;
   }
-  
+
   // Otherwise, report the backend issue as occurring in the generated .s file.
   // If Loc is invalid, we still need to report the issue, it just gets no
   // location info.
@@ -815,8 +832,8 @@ CodeGenAction::CreateASTConsumer(Compile
 LinkModules.clear();
 return nullptr;
   }
-  LinkModules.push_back(
-  {std::move(ModuleOrErr.get()), F.PropagateAttrs, F.LinkFlags});
+  LinkModules.push_back({std::move(ModuleOrErr.get()), F.PropagateAttrs,
+ F.Internalize, F.LinkFlags});
 }
 
   CoverageSourceInfo *CoverageInfo = nullptr;

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

alright. then i have a good check for myself to write ;)


https://reviews.llvm.org/D30896



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

[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.



In https://reviews.llvm.org/D28136#697844, @bader wrote:

> > Why do you think this is a bug? It seems to follow standard behavior in C 
> > to promote char to int if required. Just like if you would have a C code:
> > 
> >   int as_int(int i);
> >   void foo() {
> >   char src = 1;
> >   int dst = as_int(src);
> >   } 
> >
> > 
> > This code would complie and the same exactly IR would be generated.
>
> as_type is defined to be used for bit re-interpretation. (see 6.2.4.2 
> Reinterpreting Types Using as_type() and as_typen()). In this sense, it's 
> exactly matches __bultin_astype built-in function.
>
> Here are a few relevant OpenCL C language specification quotes from 6.2.4 
> section:
>
> > All data types described in tables 6.1 and 6.2 (except bool, half and void) 
> > may be also reinterpreted as another data type of **the same size** using 
> > the as_type() operator for scalar data types and the as_typen() operator 
> > for vector data types.
>
>
>
> > The usual type promotion for function arguments shall not be performed.
>
>
>
> > It is an error to use as_type() or as_typen() operator to reinterpret data 
> > to a type of a different number of bytes.
>
> So, aliasing as_type to __builtin_astype provides these checks, whereas we 
> can't do it for overloadable as_type function declarations.
>
> I also would like to address your original concerns:
>
> > The main issue is after preprocessing the header the original function name 
> > is no longer available in diagnostics reported.
>
> Actually diagnostics is able to provide a hint to exact code location in the 
> header file where as_ is defined as macro, so user gets correct name of 
> the operator in diagnostics as far as I know.
>
> > The spec defines as_type as a builtin function and not a macro.
>
> To be precise, spec defines as_type as an operator. So, the best way to 
> implement it would be to add a language support of such operator, but AFAIK 
> aliasing to __bulitin_astype via macro is sufficient enough for OpenCL use 
> cases.
>
> > Additionally your patch would allow as_type to be used with extra type (not 
> > only those defined in spec).
>
> Not sure I get this. We defined limited set of as_* functions - only for 
> types from tables 6.1 and 6.2 as specified by specification, so if OpenCL 
> developer will try to call as_(type2), which is not defined in the 
> header, compiler will report en error about calling undeclared function.
>
> > Also I don't see the problem to implement as_type with just simply calling 
> > a builtin. It should be inlined later anyways.
>
> Yes, but this solution will not give us error checking as pre-processor 
> solution.
>
> Does it make sense?


From all the above arguments, I feel like the right approach would be to 
implement it as Clang builtin which closely matches the operator semantic in my 
opinion. We could of course reuse the implementation of  __bultin_astype to 
avoid unnecessary extra work and code duplication.

Using the macro seems to me more like a workaround solution and overloaded 
functions don't seem to be entirely the right thing either.  What do you think 
about it?


https://reviews.llvm.org/D28136



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

r297654 - Widen bitfield for type specifiers for OpenCL types

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Mar 13 13:42:30 2017
New Revision: 297654

URL: http://llvm.org/viewvc/llvm-project?rev=297654&view=rev
Log:
Widen bitfield for type specifiers for OpenCL types

Added a static_assert to catch this issue at compile time.

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

Modified: cfe/trunk/include/clang/Basic/Specifiers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Specifiers.h?rev=297654&r1=297653&r2=297654&view=diff
==
--- cfe/trunk/include/clang/Basic/Specifiers.h (original)
+++ cfe/trunk/include/clang/Basic/Specifiers.h Mon Mar 13 13:42:30 2017
@@ -82,11 +82,12 @@ namespace clang {
   /// \brief Structure that packs information about the type specifiers that
   /// were written in a particular type specifier sequence.
   struct WrittenBuiltinSpecs {
-/*DeclSpec::TST*/ unsigned Type  : 5;
+static_assert(TST_error < 1 << 6, "Type bitfield not wide enough for TST");
+/*DeclSpec::TST*/ unsigned Type  : 6;
 /*DeclSpec::TSS*/ unsigned Sign  : 2;
 /*DeclSpec::TSW*/ unsigned Width : 2;
 unsigned ModeAttr : 1;
-  };  
+  };
 
   /// \brief A C++ access specifier (public, private, protected), plus the
   /// special value "none" which means different things in different contexts.


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

[PATCH] D30743: enable -save-temps with -finclude-defult-header

[Guansong Zhang via Phabricator via cfe-commits]

guansong updated this revision to Diff 91600.

https://reviews.llvm.org/D30743

Files:
  lib/Driver/Tools.cpp
  test/Driver/include-default-header.cl


Index: test/Driver/include-default-header.cl
===
--- /dev/null
+++ test/Driver/include-default-header.cl
@@ -0,0 +1,4 @@
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -target amdgcn -S -c %s
+
+void test() {}
+
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5285,7 +5285,20 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {
+// -finclude-default-header flag is for preprocessor,
+// do not pass it to other cc1 commands when save-temps is enabled
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {
+  Arg->claim();
+  if (StringRef(Arg->getValue()) != "-finclude-default-header")
+CmdArgs.push_back(Arg->getValue());
+}
+  }
+  else {
+Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  }
+
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
 A->claim();
 


Index: test/Driver/include-default-header.cl
===
--- /dev/null
+++ test/Driver/include-default-header.cl
@@ -0,0 +1,4 @@
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang -finclude-default-header -target amdgcn -S -c %s
+
+void test() {}
+
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5285,7 +5285,20 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  if (C.getDriver().isSaveTempsEnabled() &&
+  !isa(JA)) {
+// -finclude-default-header flag is for preprocessor,
+// do not pass it to other cc1 commands when save-temps is enabled
+for (auto Arg : Args.filtered(options::OPT_Xclang)) {
+  Arg->claim();
+  if (StringRef(Arg->getValue()) != "-finclude-default-header")
+CmdArgs.push_back(Arg->getValue());
+}
+  }
+  else {
+Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  }
+
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
 A->claim();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r297655 - Modules: Use hash of PCM content for SIGNATURE

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Mon Mar 13 13:45:08 2017
New Revision: 297655

URL: http://llvm.org/viewvc/llvm-project?rev=297655&view=rev
Log:
Modules: Use hash of PCM content for SIGNATURE

Change ASTFileSignature from a random 32-bit number to the hash of the
PCM content.

  - Move definition ASTFileSignature to Basic/Module.h so Module and
ASTSourceDescriptor can use it.

  - Change the signature from uint64_t to std::array.

  - Stop using (saving/reading) the size and modification time of PCM
files when there is a valid SIGNATURE.

  - Add UNHASHED_CONTROL_BLOCK, and use it to store the SIGNATURE record
and other records that shouldn't affect the hash.  Because implicit
modules reuses the same file for multiple levels of -Werror, this
includes DIAGNOSTIC_OPTIONS and DIAG_PRAGMA_MAPPINGS.

This helps to solve a PCH + implicit Modules dependency issue: PCH files
are handled by the external build system, whereas implicit modules are
handled by internal compiler build system.  This prevents invalidating a
PCH when the compiler overwrites a PCM file with the same content
(modulo the diagnostic differences).

Design and original patch by Manman Ren!

Modified:
cfe/trunk/include/clang/AST/ExternalASTSource.h
cfe/trunk/include/clang/Basic/Module.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/include/clang/Serialization/Module.h
cfe/trunk/lib/Basic/Module.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/GeneratePCH.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/test/Modules/diagnostic-options-out-of-date.m
cfe/trunk/test/Modules/module_file_info.m
cfe/trunk/test/Modules/rebuild.m

Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=297655&r1=297654&r2=297655&view=diff
==
--- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTSource.h Mon Mar 13 13:45:08 2017
@@ -150,20 +150,20 @@ public:
 StringRef PCHModuleName;
 StringRef Path;
 StringRef ASTFile;
-uint64_t Signature = 0;
+ASTFileSignature Signature;
 const Module *ClangModule = nullptr;
 
   public:
 ASTSourceDescriptor(){};
 ASTSourceDescriptor(StringRef Name, StringRef Path, StringRef ASTFile,
-uint64_t Signature)
+ASTFileSignature Signature)
 : PCHModuleName(std::move(Name)), Path(std::move(Path)),
   ASTFile(std::move(ASTFile)), Signature(Signature){};
 ASTSourceDescriptor(const Module &M);
 std::string getModuleName() const;
 StringRef getPath() const { return Path; }
 StringRef getASTFile() const { return ASTFile; }
-uint64_t getSignature() const { return Signature; }
+ASTFileSignature getSignature() const { return Signature; }
 const Module *getModuleOrNull() const { return ClangModule; }
   };
 

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=297655&r1=297654&r2=297655&view=diff
==
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Mon Mar 13 13:45:08 2017
@@ -42,7 +42,17 @@ class IdentifierInfo;
   
 /// \brief Describes the name of a module.
 typedef SmallVector, 2> ModuleId;
-  
+
+/// The signature of a module, which is a hash of the AST content.
+struct ASTFileSignature : std::array {
+  ASTFileSignature(std::array S = {{0}})
+  : std::array(std::move(S)) {}
+
+  explicit operator bool() const {
+return *this != std::array({{0}});
+  }
+};
+
 /// \brief Describes a module or submodule.
 class Module {
 public:
@@ -65,7 +75,7 @@ public:
   llvm::PointerUnion Umbrella;
 
   /// \brief The module signature.
-  uint64_t Signature;
+  ASTFileSignature Signature;
 
   /// \brief The name of the umbrella entry, as written in the module map.
   std::string UmbrellaAsWritten;

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=297655&r1=297654&r2=297655&view=diff
===

[PATCH] D27689: Module: hash the pcm content and use it as SIGNATURE for implicit modules.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith marked 3 inline comments as done.
dexonsmith added a comment.

Thanks for the review Richard!  Committed in r297655 with those changes.

Sorry for the long delay on this.  Somehow I missed until today the review 
after my ultimate ping.  I'd like to blame Phab, but it's more likely that I 
just failed at email :/.


https://reviews.llvm.org/D27689



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

[PATCH] D30896: [CLANG-TIDY] add check misc-prefer-switch-for-enums

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:96
+
+  Finds uses of enumeration values in equality and inequality expressions 
where a switch would be preferred.
+

Please highlight switch with `` and indent preferred on same column as 
beginning of sentence.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst:8
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value

Please highlight switch with ``.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst:10
+warning. No such warning exists for control flow based on enum value
+comparison.  Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for

Please highlight if with ``.


https://reviews.llvm.org/D30896



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

LLVM Lab SVN mirror is behind

[Galina Kistanova via cfe-commits]

Hello everyone,

The SVN mirror in the LLVM Lab seems behind with the changes.
I'm looking at the issue.

Thank you for understanding.

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

[PATCH] D30810: Preserve vec3 type.

[JinGu Kang via Phabricator via cfe-commits]

jaykang10 added a comment.

In https://reviews.llvm.org/D30810#699428, @Anastasia wrote:

> Would you be able to update ScalarExprEmitter::VisitAsTypeExpr implementation 
> accordingly to make things consistent?


Probably, No... the load/store with vec3 generates vec4 temporarily to be 
aligned but __builtin_astype needs to generate vec4 from vec3  or vice versa 
because spec allows it on 6.2.4.2. As we discussed on previous e-mails, we 
would need to add intrinsic function for __builtin_astype on llvm and add 
default behavior on llvm's codegen. It is beyond clang and I am not sure llvm's 
IR and codegen people allow it... If you have another idea or I missed 
something, please let me know.


https://reviews.llvm.org/D30810



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

r297659 - AMDGPU: Make 0 the private nullptr value

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Mon Mar 13 14:47:53 2017
New Revision: 297659

URL: http://llvm.org/viewvc/llvm-project?rev=297659&view=rev
Log:
AMDGPU: Make 0 the private nullptr value

We can't actually pretend that 0 is valid for address space 0.
r295877 added a workaround to stop allocating user objects
there, so we can use 0 as the invalid pointer.

Some of the tests seemed to be using private as the non-0 null
test address space, so add copies using local to make sure
this is still stressed.

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
cfe/trunk/test/CodeGenOpenCL/blocks.cl

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=297659&r1=297658&r2=297659&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Mon Mar 13 14:47:53 2017
@@ -2298,7 +2298,7 @@ public:
   // address space has value 0 but in private and local address space has
   // value ~0.
   uint64_t getNullPointerValue(unsigned AS) const override {
-return AS != LangAS::opencl_local && AS != 0 ? 0 : ~0;
+return AS == LangAS::opencl_local ? ~0 : 0;
   }
 };
 

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl?rev=297659&r1=297658&r2=297659&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl Mon Mar 13 14:47:53 2017
@@ -20,7 +20,7 @@ typedef struct {
 
 // Test 0 as initializer.
 
-// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* 
addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// CHECK: @private_p = local_unnamed_addr addrspace(1) global i8* null, align 4
 private char *private_p = 0;
 
 // CHECK: @local_p = local_unnamed_addr addrspace(1) global i8 addrspace(3)* 
addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
@@ -37,7 +37,7 @@ generic char *generic_p = 0;
 
 // Test NULL as initializer.
 
-// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* 
addrspacecast (i8 addrspace(4)* null to i8*), align 4
+// CHECK: @private_p_NULL = local_unnamed_addr addrspace(1) global i8* null, 
align 4
 private char *private_p_NULL = NULL;
 
 // CHECK: @local_p_NULL = local_unnamed_addr addrspace(1) global i8 
addrspace(3)* addrspacecast (i8 addrspace(4)* null to i8 addrspace(3)*), align 4
@@ -58,38 +58,55 @@ generic char *generic_p_NULL = NULL;
 // CHECK: @fold_generic = local_unnamed_addr addrspace(1) global i32 
addrspace(4)* null, align 4
 generic int *fold_generic = (global int*)(generic float*)(private char*)0;
 
-// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* 
addrspacecast (i16 addrspace(4)* null to i16*), align 4
+// CHECK: @fold_priv = local_unnamed_addr addrspace(1) global i16* null, align 
4
 private short *fold_priv = (private short*)(generic int*)(global void*)0;
 
-// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* 
inttoptr (i32 9 to i8*), align 4
+// CHECK: @fold_priv_arith = local_unnamed_addr addrspace(1) global i8* 
inttoptr (i32 10 to i8*), align 4
 private char *fold_priv_arith = (private char*)0 + 10;
 
-// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 13, align 4
+// CHECK: @fold_int = local_unnamed_addr addrspace(1) global i32 14, align 4
 int fold_int = (int)(private void*)(generic char*)(global int*)0 + 14;
 
-// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 12, align 4
+// CHECK: @fold_int2 = local_unnamed_addr addrspace(1) global i32 13, align 4
 int fold_int2 = (int) ((private void*)0 + 13);
 
-// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 -1, align 4
+// CHECK: @fold_int3 = local_unnamed_addr addrspace(1) global i32 0, align 4
 int fold_int3 = (int) ((private int*)0);
 
-// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 7, align 4
+// CHECK: @fold_int4 = local_unnamed_addr addrspace(1) global i32 8, align 4
 int fold_int4 = (int) &((private int*)0)[2];
 
-// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 3, align 4
+// CHECK: @fold_int5 = local_unnamed_addr addrspace(1) global i32 4, align 4
 int fold_int5 = (int) &((private StructTy1*)0)->p2;
 
+
+// CHECK: @fold_int_local = local_unnamed_addr addrspace(1) global i32 13, 
align 4
+int fold_int_local = (int)(local void*)(generic char*)(global int*)0 + 14;
+
+// CHECK: @fold_int2_local = local_unnamed_addr addrspace(1) global i32 12, 
align 4
+int fold_int2_local = (int) ((local void*)0 + 13);
+
+// CHECK: @fold_int3_local = local_unnamed_addr addrspace(1) global i32 -1, 
align 4
+int fold_int3_local = (int) ((local int*)0);
+
+// CHECK: @fold_int4_local = local_unnamed_addr addrspace(1) global i32 7, 
align 4
+int fold_

[PATCH] D30316: AMDGPU: Make 0 the private nullptr value

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r297658, r297659


https://reviews.llvm.org/D30316



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

[PATCH] D30810: Preserve vec3 type.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a reviewer: bruno.
bruno added a comment.

Hi JinGu,

I just read the discussion on cfe-dev, some comments:

> My colleague and I are implementing a transformation pass between LLVM IR and 
> another IR and we want to keep the 3-component vector types in our target IR

Why can't you go back through the shuffle to find out that it comes from a vec3 
-> vec4 transformation? Adding a flag here just for that seems very odd.


https://reviews.llvm.org/D30810



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

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Hi Pete, thanks for working on this!

LGTM with the minor comments below.




Comment at: include/clang/Lex/PPCallbacks.h:264
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }
   

clang-format this one!



Comment at: test/Frontend/dependency-gen-has-include.c:17
+#if __has_include("a/header.h")
+#endif

Can you also add a testcase for `__has_include_next`?


https://reviews.llvm.org/D30882



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

[PATCH] D30881: Track skipped files in dependency scanning

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Pete,




Comment at: lib/Frontend/DependencyFile.cpp:191
+   const Token &FilenameTok,
+   SrcMgr::CharacteristicKind FileType) override;
+

Is there any `FileSkipped` callback invocation that might trigger an unwanted 
file to be added as a dependency or it will only trigger for the symlink case?



Comment at: test/Frontend/dependency-gen-symlink.c:11
+// RUN: echo "#endif" >> %t.dir/a/header.h
+// RUN: ln %t.dir/a/header.h %t.dir/b/header.h
+

It seems that it works for hard links as well right? Is this intended or do you 
miss a `-s`?


https://reviews.llvm.org/D30881



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

[PATCH] D29877: Warn about unused static file scope function template declarations.

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/Sema/Sema.cpp:472-477
+// If this is a function template, we should remove if it has no
+// specializations.
+if (FunctionTemplateDecl *Template = FD->getDescribedFunctionTemplate()) {
+  if (std::distance(Template->spec_begin(), Template->spec_end()))
+return true;
+}

The comment doesn't match the code: you're removing function templates if they 
/do/ have specializations. And I think we should probably be walking the list 
of specializations and considering the template to be used if any 
specialization is used. That would affect a case like:

```
template static void f() {}
template<> static void f() {}
```

... where the primary template is still unused despite having a specialization.



Comment at: lib/Sema/Sema.cpp:492
 
   if (const VarDecl *VD = dyn_cast(D)) {
 // If a variable usable in constant expressions is referenced,

Should we do the same thing for variable templates?



Comment at: lib/Sema/SemaDecl.cpp:1496
 return false;
+  // 'static operator' functions are defined in headers; don't warn.
+  if (FD->isOverloadedOperator() &&

Why? Defining a static operator in a header sounds like a bug to me.


https://reviews.llvm.org/D29877



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

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich created this revision.

This is the second part of https://reviews.llvm.org/D28445, it extends taint 
propagation to cases where the tainted region is a sub-region and we can't 
taint a conjured symbol entirely. This required adding a new map in the GDM 
that maps tainted parent symbols to tainted sub-regions (in order to avoid a 
linear scan looking for appropriate symbols in the current TaintMap.) With this 
change, tainting of structs and unions should work as expected.


https://reviews.llvm.org/D30909

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -204,8 +204,10 @@
   sock = socket(AF_INET, SOCK_STREAM, 0);
   read(sock, &tainted.buf[0], sizeof(tainted.buf));
   read(sock, &tainted.st[0], sizeof(tainted.st));
-  // FIXME: tainted.st[0].length should be marked tainted
-  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // no-warning
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+
+  read(sock, &tainted.st, sizeof(tainted.st));
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
 }
 
 int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -496,7 +496,10 @@
 
   Optional getDefaultBinding(Store S, const MemRegion *R) override {
 RegionBindingsRef B = getRegionBindings(S);
-return B.getDefaultBinding(R);
+// Default bindings are always applied over a base region so look up the
+// base region's default binding, otherwise the lookup will fail when R
+// is at an offset from R->getBaseRegion().
+return B.getDefaultBinding(R->getBaseRegion());
   }
 
   SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -671,6 +671,17 @@
 
   ProgramStateRef NewState = set(Sym, Kind);
   assert(NewState);
+
+  if (const SymbolDerived *SD = dyn_cast(Sym)) {
+TaintedSymRegionsRef SymRegions(0, TSRFactory.getTreeFactory());
+if (contains(SD->getParentSymbol()))
+  SymRegions = *get(SD->getParentSymbol());
+
+SymRegions = SymRegions.add(SD->getRegion());
+NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+  }
+
+  assert(NewState);
   return NewState;
 }
 
@@ -723,15 +734,31 @@
 const TaintTagType *Tag = get(*SI);
 Tainted = (Tag && *Tag == Kind);
 
-// If this is a SymbolDerived with a tainted parent, it's also tainted.
-if (const SymbolDerived *SD = dyn_cast(*SI))
+if (const SymbolDerived *SD = dyn_cast(*SI)) {
+  // If this is a SymbolDerived with a tainted parent, it's also tainted.
   Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
 
+  // If this is a SymbolDerived with the same parent symbol as another
+  // tainted SymbolDerived and a region that's a sub-region of that tainted
+  // symbol, it's also tainted.
+  if (contains(SD->getParentSymbol())) {
+const TypedValueRegion *R = SD->getRegion();
+const TaintedSymRegionsRef *SymRegions =
+get(SD->getParentSymbol());
+for (TaintedSymRegionsRef::iterator I = SymRegions->begin(),
+E = SymRegions->end();
+ I != E; ++I) {
+  if (R == *I || R->isSubRegionOf(*I))
+return true;
+}
+  }
+}
+
 // If memory region is tainted, data is also tainted.
 if (const SymbolRegionValue *SRV = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SRV->getRegion(), Kind);
 
-// If If this is a SymbolCast from a tainted value, it's also tainted.
+// If this is a SymbolCast from a tainted value, it's also tainted.
 if (const SymbolCast *SC = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SC->getOperand(), Kind);
 
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -72,8 +72,6 @@
   /// covers the entire region, e.g. we avoid false positives by not returning
   /// a default bindingc for an entire struct if the symbol 

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich updated this revision to Diff 91614.
vlad.tsyrklevich added a comment.

Fix a stray assert()


https://reviews.llvm.org/D30909

Files:
  lib/StaticAnalyzer/Core/ProgramState.cpp


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -679,9 +679,9 @@
 
 SymRegions = SymRegions.add(SD->getRegion());
 NewState = NewState->set(SD->getParentSymbol(), 
SymRegions);
+assert(NewState);
   }
 
-  assert(NewState);
   return NewState;
 }
 


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -679,9 +679,9 @@
 
 SymRegions = SymRegions.add(SD->getRegion());
 NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+assert(NewState);
   }
 
-  assert(NewState);
   return NewState;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich updated this revision to Diff 91615.
vlad.tsyrklevich added a comment.

Fix a stray assert(), correctly this time..


https://reviews.llvm.org/D30909

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -204,8 +204,10 @@
   sock = socket(AF_INET, SOCK_STREAM, 0);
   read(sock, &tainted.buf[0], sizeof(tainted.buf));
   read(sock, &tainted.st[0], sizeof(tainted.st));
-  // FIXME: tainted.st[0].length should be marked tainted
-  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // no-warning
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+
+  read(sock, &tainted.st, sizeof(tainted.st));
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
 }
 
 int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -496,7 +496,10 @@
 
   Optional getDefaultBinding(Store S, const MemRegion *R) override {
 RegionBindingsRef B = getRegionBindings(S);
-return B.getDefaultBinding(R);
+// Default bindings are always applied over a base region so look up the
+// base region's default binding, otherwise the lookup will fail when R
+// is at an offset from R->getBaseRegion().
+return B.getDefaultBinding(R->getBaseRegion());
   }
 
   SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -671,6 +671,17 @@
 
   ProgramStateRef NewState = set(Sym, Kind);
   assert(NewState);
+
+  if (const SymbolDerived *SD = dyn_cast(Sym)) {
+TaintedSymRegionsRef SymRegions(0, TSRFactory.getTreeFactory());
+if (contains(SD->getParentSymbol()))
+  SymRegions = *get(SD->getParentSymbol());
+
+SymRegions = SymRegions.add(SD->getRegion());
+NewState = NewState->set(SD->getParentSymbol(), SymRegions);
+assert(NewState);
+  }
+
   return NewState;
 }
 
@@ -723,15 +734,31 @@
 const TaintTagType *Tag = get(*SI);
 Tainted = (Tag && *Tag == Kind);
 
-// If this is a SymbolDerived with a tainted parent, it's also tainted.
-if (const SymbolDerived *SD = dyn_cast(*SI))
+if (const SymbolDerived *SD = dyn_cast(*SI)) {
+  // If this is a SymbolDerived with a tainted parent, it's also tainted.
   Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
 
+  // If this is a SymbolDerived with the same parent symbol as another
+  // tainted SymbolDerived and a region that's a sub-region of that tainted
+  // symbol, it's also tainted.
+  if (contains(SD->getParentSymbol())) {
+const TypedValueRegion *R = SD->getRegion();
+const TaintedSymRegionsRef *SymRegions =
+get(SD->getParentSymbol());
+for (TaintedSymRegionsRef::iterator I = SymRegions->begin(),
+E = SymRegions->end();
+ I != E; ++I) {
+  if (R == *I || R->isSubRegionOf(*I))
+return true;
+}
+  }
+}
+
 // If memory region is tainted, data is also tainted.
 if (const SymbolRegionValue *SRV = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SRV->getRegion(), Kind);
 
-// If If this is a SymbolCast from a tainted value, it's also tainted.
+// If this is a SymbolCast from a tainted value, it's also tainted.
 if (const SymbolCast *SC = dyn_cast(*SI))
   Tainted = Tainted || isTainted(SC->getOperand(), Kind);
 
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -72,8 +72,6 @@
   /// covers the entire region, e.g. we avoid false positives by not returning
   /// a default bindingc for an entire struct if the symbol for only a single
   /// field or element within it is requested.
-  // TODO: Return an appropriate symbol for sub-fields/elements of an LCV so
-  // that they are also appropriately tainted.
   static SymbolRef getLCVSymbol(CheckerContext &C,
 nonloc::LazyCompoundVal &LCV);
 
@@ -479,19 +477,21 @@
 
   // getLCVSymbol() is re

[PATCH] D30810: Preserve vec3 type.

[JinGu Kang via Phabricator via cfe-commits]

jaykang10 added a comment.

In https://reviews.llvm.org/D30810#699695, @bruno wrote:

> Hi JinGu,
>
> I just read the discussion on cfe-dev, some comments:
>
> > My colleague and I are implementing a transformation pass between LLVM IR 
> > and another IR and we want to keep the 3-component vector types in our 
> > target IR
>
> Why can't you go back through the shuffle to find out that it comes from a 
> vec3 -> vec4 transformation? Adding a flag here just for that seems very odd.


Hi Bruno,

Thanks for your comment. We have a pass to undo the vec4 to vec3. I wondered 
why clang generates the vec4 for vec3 load/store. As you can see the comment on 
clang's code, they are generated for better performance. I had a questions at 
this point. clang should consider vector load/store aligned by 4 regardless of 
target??? llvm's codegen could handle vec3 according to targets' vector 
load/store with their alignment. I agree the flag looks odd but I was concerned 
some llvm's targets depend on the vec4 so I suggested to add the flag. If I 
missed something, please let me know.


https://reviews.llvm.org/D30810



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

[PATCH] D30898: Add new -fverbose-asm that enables source interleaving

[Andrey Bokhanko via Phabricator via cfe-commits]

andreybokhanko added a comment.

Hi Roger,

I'm very glad to see you started to work on this!

A somewhat obvious comment: no chance for this to be accepted without LIT 
tests. I understand you have your doubts on the best approach to testing -- and 
it's OK to ask either here or on llvm-dev -- but tests should be added 
nevertheless.

Yours,
Andrey


https://reviews.llvm.org/D30898



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

[clang-tools-extra] r297671 - Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the cppcoreguidelines-special-member-functions check.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Mon Mar 13 16:39:00 2017
New Revision: 297671

URL: http://llvm.org/viewvc/llvm-project?rev=297671&view=rev
Log:
Add the 'AllowSoleDefaultDtor' and 'AllowMissingMoveFunctions' options to the 
cppcoreguidelines-special-member-functions check.

Patch by Florian Gross.

Added:

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
Modified:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-special-member-functions.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp?rev=297671&r1=297670&r2=297671&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
 Mon Mar 13 16:39:00 2017
@@ -22,6 +22,18 @@ namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
+SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)),
+  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {}
+
+void SpecialMemberFunctionsCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions);
+  Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor);
+}
+
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
 return;
@@ -48,6 +60,12 @@ toString(SpecialMemberFunctionsCheck::Sp
   switch (K) {
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::Destructor:
 return "a destructor";
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::
+  DefaultDestructor:
+return "a default destructor";
+  case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::
+  NonDefaultDestructor:
+return "a non-default destructor";
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyConstructor:
 return "a copy constructor";
   case SpecialMemberFunctionsCheck::SpecialMemberFunctionKind::CopyAssignment:
@@ -88,51 +106,86 @@ void SpecialMemberFunctionsCheck::check(
 
   ClassDefId ID(MatchedDecl->getLocation(), MatchedDecl->getName());
 
+  auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) {
+llvm::SmallVectorImpl &Members =
+ClassWithSpecialMembers[ID];
+if (!llvm::is_contained(Members, Kind))
+  Members.push_back(Kind);
+  };
+
+  if (const auto *Dtor = Result.Nodes.getNodeAs("dtor")) {
+StoreMember(Dtor->isDefaulted()
+? SpecialMemberFunctionKind::DefaultDestructor
+: SpecialMemberFunctionKind::NonDefaultDestructor);
+  }
+
   std::initializer_list>
-  Matchers = {{"dtor", SpecialMemberFunctionKind::Destructor},
-  {"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
+  Matchers = {{"copy-ctor", SpecialMemberFunctionKind::CopyConstructor},
   {"copy-assign", SpecialMemberFunctionKind::CopyAssignment},
   {"move-ctor", SpecialMemberFunctionKind::MoveConstructor},
   {"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
 
   for (const auto &KV : Matchers)
 if (Result.Nodes.getNodeAs(KV.first)) {
-  SpecialMemberFunctionKind Kind = KV.second;
-  llvm::SmallVectorImpl &Members =
-  ClassWithSpecialMembers[ID];
-  if (find(Members, Kind) == Members.end())
-Members.push_back(Kind);
+  StoreMember(KV.second);
 }
 }
 
 void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
-  llvm::SmallVector AllSpecialMembers = {
-  SpecialMemberFunctionKind::Destructor,
-  SpecialMemberFunctionKind::CopyConstructor,
-  SpecialMemberFunctionKind::CopyAssignment};
-
-  if (getLangOpts().CPlusPlus11) {
-AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveConstructor);
-AllSpecialMembers.push_back(SpecialMemberFunctionKind::MoveAssignment);
+  for (const auto &C : ClassWithSpecialMembers) {
+checkForMissingMembers(C.first, C.second);
   }
+}
 
-  for (const auto &C : ClassWithSpecialMembers) {
-const auto &DefinedSpecialMembers = C.second;
+void SpecialMemberFunctionsCheck::checkForMissingMembers(
+const ClassD

[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit in r297671, thanks!


https://reviews.llvm.org/D30610



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

[PATCH] D29923: PPCallbacks::MacroUndefined, change signature and add test.

[Frederich Munch via Phabricator via cfe-commits]

marsupial updated this revision to Diff 91627.
marsupial retitled this revision from "Add test for 
PPCallbacks::MacroUndefined" to "PPCallbacks::MacroUndefined, change signature 
and add test.".
marsupial edited the summary of this revision.
Herald added a subscriber: nemanjai.

https://reviews.llvm.org/D29923

Files:
  include/clang/Lex/PPCallbacks.h
  include/clang/Lex/PreprocessingRecord.h
  lib/Frontend/PrintPreprocessedOutput.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/PreprocessingRecord.cpp
  tools/libclang/Indexing.cpp
  unittests/Basic/SourceManagerTest.cpp

Index: unittests/Basic/SourceManagerTest.cpp
===
--- unittests/Basic/SourceManagerTest.cpp
+++ unittests/Basic/SourceManagerTest.cpp
@@ -246,12 +246,18 @@
 namespace {
 
 struct MacroAction {
+  enum Kind { kExpansion, kDefinition, kUnDefinition};
+
   SourceLocation Loc;
   std::string Name;
-  bool isDefinition; // if false, it is expansion.
-  
-  MacroAction(SourceLocation Loc, StringRef Name, bool isDefinition)
-: Loc(Loc), Name(Name), isDefinition(isDefinition) { }
+  unsigned MAKind : 3;
+
+  MacroAction(SourceLocation Loc, StringRef Name, unsigned K)
+: Loc(Loc), Name(Name), MAKind(K) { }
+
+  bool isExpansion() const { return MAKind == kExpansion; }
+  bool isDefinition() const { return MAKind & kDefinition; }
+  bool isUnDefinition() const { return MAKind & kUnDefinition; }
 };
 
 class MacroTracker : public PPCallbacks {
@@ -264,21 +270,33 @@
 const MacroDirective *MD) override {
 Macros.push_back(MacroAction(MD->getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- true));
+ MacroAction::kDefinition));
+  }
+  void MacroUndefined(const Token &MacroNameTok,
+  const MacroDefinition &MD,
+  const MacroDirective  *UD) override {
+Macros.push_back(
+MacroAction(UD ? UD->getLocation() : SourceLocation(),
+MacroNameTok.getIdentifierInfo()->getName(),
+UD ? MacroAction::kDefinition | MacroAction::kUnDefinition
+   : MacroAction::kUnDefinition));
   }
   void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
 SourceRange Range, const MacroArgs *Args) override {
 Macros.push_back(MacroAction(MacroNameTok.getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
- false));
+ MacroAction::kExpansion));
   }
 };
 
 }
 
 TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   const char *header =
-"#define MACRO_IN_INCLUDE 0\n";
+"#define MACRO_IN_INCLUDE 0\n"
+"#define MACRO_DEFINED\n"
+"#undef MACRO_DEFINED\n"
+"#undef MACRO_UNDEFINED\n";
 
   const char *main =
 "#define M(x) x\n"
@@ -323,42 +341,54 @@
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(0U, toks.size());
 
-  ASSERT_EQ(9U, Macros.size());
+  ASSERT_EQ(15U, Macros.size());
   // #define M(x) x
-  ASSERT_TRUE(Macros[0].isDefinition);
+  ASSERT_TRUE(Macros[0].isDefinition());
   ASSERT_EQ("M", Macros[0].Name);
   // #define INC "/test-header.h"
-  ASSERT_TRUE(Macros[1].isDefinition);
+  ASSERT_TRUE(Macros[1].isDefinition());
   ASSERT_EQ("INC", Macros[1].Name);
   // M expansion in #include M(INC)
-  ASSERT_FALSE(Macros[2].isDefinition);
+  ASSERT_FALSE(Macros[2].isDefinition());
   ASSERT_EQ("M", Macros[2].Name);
   // INC expansion in #include M(INC)
-  ASSERT_FALSE(Macros[3].isDefinition);
+  ASSERT_TRUE(Macros[3].isExpansion());
   ASSERT_EQ("INC", Macros[3].Name);
   // #define MACRO_IN_INCLUDE 0
-  ASSERT_TRUE(Macros[4].isDefinition);
+  ASSERT_TRUE(Macros[4].isDefinition());
   ASSERT_EQ("MACRO_IN_INCLUDE", Macros[4].Name);
+  // #define MACRO_DEFINED
+  ASSERT_TRUE(Macros[5].isDefinition());
+  ASSERT_FALSE(Macros[5].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[5].Name);
+  // #undef MACRO_DEFINED
+  ASSERT_TRUE(Macros[6].isDefinition());
+  ASSERT_TRUE(Macros[6].isUnDefinition());
+  ASSERT_EQ("MACRO_DEFINED", Macros[6].Name);
+  // #undef MACRO_UNDEFINED
+  ASSERT_FALSE(Macros[7].isDefinition());
+  ASSERT_TRUE(Macros[7].isUnDefinition());
+  ASSERT_EQ("MACRO_UNDEFINED", Macros[7].Name);
   // #define INC2 
-  ASSERT_TRUE(Macros[5].isDefinition);
-  ASSERT_EQ("INC2", Macros[5].Name);
+  ASSERT_TRUE(Macros[8].isDefinition());
+  ASSERT_EQ("INC2", Macros[8].Name);
   // M expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[6].isDefinition);
-  ASSERT_EQ("M", Macros[6].Name);
+  ASSERT_FALSE(Macros[9].isDefinition());
+  ASSERT_EQ("M", Macros[9].Name);
   // INC2 expansion in #include M(INC2)
-  ASSERT_FALSE(Macros[7].isDefinition);
-  ASSERT_EQ("INC2", Macros[7].Name);
+  ASSERT_TRUE(Macros[10].isExpansion());
+  ASSERT_EQ("INC2", Macros[10].Name);
   // #de

Re: [libcxx] r297553 - Change test coverage generation to use llvm-cov instead of gcov.

[Bruno Cardoso Lopes via cfe-commits]

Hi Eric,

>  if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
> @@ -62,12 +66,7 @@ if (APPLE AND LLVM_USE_SANITIZER)
>  message(WARNING "LLVM_USE_SANITIZER=${LLVM_USE_SANITIZER} is not 
> supported on OS X")
>endif()
>if (LIBFILE)
> -execute_process(COMMAND ${CMAKE_CXX_COMPILER} -print-file-name=lib 
> OUTPUT_VARIABLE LIBDIR RESULT_VARIABLE Result)
> -if (NOT ${Result} EQUAL "0")
> -  message(FATAL "Failed to find library resource directory")
> -endif()
> -string(STRIP "${LIBDIR}" LIBDIR)
> -set(LIBDIR "${LIBDIR}/darwin/")
> +find_compiler_rt_dir(LIBDIR)

Seems like this broke ASAN+UBSAN bot in green-dragon:
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan_build/3812

Perhaps it's missing the "${LIBDIR}/darwin/" part from above?

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.h:20
+///\brief Warns about using throw in function declared as noexcept.
+/// It complains about every throw, even if it is caught later.
+class ThrowWithNoexceptCheck : public ClangTidyCheck {

sbarzowski wrote:
> sbarzowski wrote:
> > vmiklos wrote:
> > > Prazek wrote:
> > > > aaron.ballman wrote:
> > > > > What is the false positive rate with this check over a large codebase 
> > > > > that uses exceptions?
> > > > Do you know any good project to check it?
> > > LibreOffice might be a candidate, see 
> > >  for details 
> > > on how to generate a compile database for it (since it does not use 
> > > cmake), then you can start testing.
> > Thanks. However it's not just about using exception. To be meaningful I 
> > need a project that use noexcept in more than a few places.
> > 
> > Here are some projects that I found:
> > https://github.com/facebook/hhvm/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/facebook/folly/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/Tencent/mars/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/facebook/rocksdb/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/CRYTEK-CRYENGINE/CRYENGINE/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/SFTtech/openage/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/facebook/watchman/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/facebook/proxygen/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/philsquared/Catch/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/sandstorm-io/capnproto/search?utf8=%E2%9C%93&q=noexcept
> > https://github.com/miloyip/rapidjson/search?utf8=%E2%9C%93&q=noexcept
> > 
> > I've tried HHVM so far, and ran into some problems compiling it. Anyway I'm 
> > working on it.
> Ok, I was able to run it on most of the HHVM  codebase + deps. There were 
> some issues that looked like some autogenerated pieces missing, so it may 
> have been not 100% covered.
> 
> The results:
> 3 occurences in total
> 1) rethrow in destructor (http://pastebin.com/JRNMZGev)
> 2) false positive "throw and catch in the same function" 
> (http://pastebin.com/14y1AJgM)
> 3) noexcept decided during compilation (http://pastebin.com/1jZzRAbC)
> 
> That's not too many, but this is a kind of check that I guess would be most 
> useful earlier in the development - ideally before the initial code review.
> 
> I'm not sure if we should count (3) as false positive. We could potentially 
> eliminate it, by checking if the expression in noexcept is empty or true 
> literal.
> 
> (2) is def. a false positive. The potential handling of this case was already 
> proposed, but I'm not sure if it's worth it.  The code in example (2) is ugly 
> and extracting these throwing parts to separate functions looks like a good 
> way to start refactoring. 
> 
> What do you think?
> 
The fact that there's a false positive in the first large project you checked 
suggests that the false positive is likely worth it to fix. The code may be 
ugly, but it's not uncommon -- some coding guidelines explicitly disallow use 
of gotos, and this is a reasonable(ish) workaround for that.

As for #3, I would consider that to be a false-positive as well. A computed 
noexcept clause is going to be very common, especially in generic code. I think 
it's probably more important to get this one right than #2.


https://reviews.llvm.org/D19201



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

[PATCH] D30915: Canonicalize the path provided by -fmodules-cache-path

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.

This fixes lookup mismatches that could previously happen when the module cache 
path contained a '/./' component.
In combination with https://reviews.llvm.org/D28299 this bug can cause a 
use-after-free.

rdar://problem/30413458


https://reviews.llvm.org/D30915

Files:
  lib/Frontend/CompilerInvocation.cpp
  test/Modules/Inputs/outofdate-rebuild/AppKit.h
  test/Modules/Inputs/outofdate-rebuild/Cocoa.h
  test/Modules/Inputs/outofdate-rebuild/CoreText.h
  test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
  test/Modules/Inputs/outofdate-rebuild/Foundation.h
  test/Modules/Inputs/outofdate-rebuild/module.modulemap
  test/Modules/modules-cache-path-canonicalization.m

Index: test/Modules/modules-cache-path-canonicalization.m
===
--- /dev/null
+++ test/Modules/modules-cache-path-canonicalization.m
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t/cache %T/rel
+
+// This testcase reproduces a use-after-free after looking up a PCM in
+// a non-canonical modules-cache-path.
+//
+// Prime the module cache (note the '.' in the path).
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+//
+// Force a module to be rebuilt by creating a conflict.
+// RUN: echo "@import CoreText;" > %t.m
+// RUN: %clang_cc1 -DMISMATCH -Werror -fdisable-module-hash \
+// RUN:   -fmodules-cache-path=%t/./cache -fmodules -fimplicit-module-maps \
+// RUN:   -I %S/Inputs/outofdate-rebuild %t.m -fsyntax-only
+//
+// Rebuild.
+// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   %s -fsyntax-only
+
+
+// Unrelated to the above: Check that a relative path is resolved correctly.
+//
+// RUN: %clang_cc1 -working-directory %T/rel -fmodules-cache-path=./cache \
+// RUN:   -fmodules -fimplicit-module-maps -I %S/Inputs/outofdate-rebuild \
+// RUN:   -fdisable-module-hash %t.m -fsyntax-only -Rmodule-build 2>&1 \
+// RUN:   | FileCheck %s
+// CHECK: /rel/cache/CoreText.pcm
+@import Cocoa;
Index: test/Modules/Inputs/outofdate-rebuild/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/module.modulemap
@@ -0,0 +1,19 @@
+module Cocoa {
+  header "Cocoa.h"
+}
+
+module AppKit {
+  header "AppKit.h"
+}
+
+module CoreText {
+  header "CoreText.h"
+}
+
+module Foundation {
+  header "Foundation.h"
+}
+
+module CoreVideo {
+  header "CoreVideo.h"
+}
Index: test/Modules/Inputs/outofdate-rebuild/Foundation.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/Foundation.h
@@ -0,0 +1,3 @@
+// Foundation
+#import "CoreText.h"
+struct D { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/CoreVideo.h
@@ -0,0 +1,3 @@
+// CoreVideo
+#import "Foundation.h" // Foundation
+struct E { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/CoreText.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/CoreText.h
@@ -0,0 +1 @@
+struct C { int i; };
Index: test/Modules/Inputs/outofdate-rebuild/Cocoa.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/Cocoa.h
@@ -0,0 +1,5 @@
+// Cocoa
+#import "Foundation.h"
+#import "AppKit.h"
+
+struct A {  int i; };
Index: test/Modules/Inputs/outofdate-rebuild/AppKit.h
===
--- /dev/null
+++ test/Modules/Inputs/outofdate-rebuild/AppKit.h
@@ -0,0 +1,3 @@
+// AppKit
+#import "CoreVideo.h" // CoreVideo
+struct B { int i; };
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1419,7 +1419,8 @@
   return P.str();
 }
 
-static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args) {
+static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
+  const std::string &WorkingDir) {
   using namespace options;
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");
   Opts.Verbose = Args.hasArg(OPT_v);
@@ -1429,7 +1430,18 @@
   if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
 Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
-  Opts.ModuleCachePath = Args.getLastArgValue(OPT_fmodules_cache_path);
+
+  // Canonicalize -fmodules-cache-path before storing it.
+  SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));
+  if (!(P.empty() || llvm::sy

[PATCH] D30766: Add support for attribute "enum_extensibility"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:1969
+  let Content = [{
+Attribute ``enum_extensibility`` is used to distinguish between enum 
definitions that are extensible and those that are not. The attribute can take 
either ``closed`` or ``open`` as an argument. ``closed`` indicates a variable 
of the enum type takes a value that corresponds to one of the enumerators 
listed in the enum definition or, when the enum is annotated with 
``flag_enum``, a value that can be constructed using values corresponding to 
the enumerators. ``open`` indicates a variable of the enum type can take any 
values allowed by the standard and instructs clang to be more lenient when 
issuing warnings.
+  }];

Breaking this up into multiple lines would be appreciated (we aren't strict 
about the 80 col limit in this file, but it should be readable still).

Also, adding some examples would be appreciated.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2383-2384
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_not_supported : Error<
+  "%0 attribute argument not supported: %1">;
 def err_init_priority_object_attr : Error<

I'd rather see this defined next to `warn_attribute_type_not_supported` as `def 
err_attribute_type_not_supported : 
Error;`



Comment at: test/Sema/enum-attr.c:27
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // 
expected-warning{{'enum_extensibility' attribute argument not supported: 
'arg1'}}
+  G

ahatanak wrote:
> arphaman wrote:
> > Should this be an error instead?
> Yes, I agree.
I'm not opposed to it, but we do treat it as a warning for every other 
attribute (and just ignore the attribute), FWIW.



Comment at: test/SemaCXX/enum-attr.cpp:108
+  }
+}

Missing tests for the attribute being applied to something other than an enum, 
or taking too few/many arguments.


https://reviews.llvm.org/D30766



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

[PATCH] D30915: Canonicalize the path provided by -fmodules-cache-path

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM




Comment at: lib/Frontend/CompilerInvocation.cpp:1434
+
+  // Canonicalize -fmodules-cache-path before storing it.
+  SmallString<128> P(Args.getLastArgValue(OPT_fmodules_cache_path));

I would suggest using `FileManager::makeAbsolutePath`, but at this point looks 
like we haven't set up a FileManager yet.


https://reviews.llvm.org/D30915



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

[PATCH] D30896: [Clang-tidy] add check misc-prefer-switch-for-enums

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm curious what kind of results you get when running this over a large code 
base. There are definitely times when using == or != for a specific value is 
*way* cleaner than using a switch statement, and I worry about this being so 
chatty that it needs to be disabled by default.

If it turns out to be very chatty, perhaps the check could be relaxed to only 
consider cases where you have multiple if/else if clauses (tuned via an option) 
for checking several values? At the very least, the check should not be 
triggered on an enumeration that has only one enumerator.


Repository:
  rL LLVM

https://reviews.llvm.org/D30896



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

[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama updated this revision to Diff 91633.
pirama added a comment.

- Rebase
- Added command line flag and updated tests.


https://reviews.llvm.org/D30700

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/arch-specific-libdir-rpath.c
  test/Driver/arch-specific-libdir.c
  test/lit.cfg

Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -397,10 +397,6 @@
 if config.host_triple == config.target_triple:
 config.available_features.add("native")
 
-# Test Driver/arch-specific-libdir-rpath.c is restricted to x86_64-linux
-if re.match(r'^x86_64.*-linux', config.target_triple):
-config.available_features.add("x86_64-linux")
-
 # Case-insensitive file system
 def is_filesystem_case_insensitive():
 handle, path = tempfile.mkstemp(prefix='case-test', dir=config.test_exec_root)
Index: test/Driver/arch-specific-libdir.c
===
--- test/Driver/arch-specific-libdir.c
+++ test/Driver/arch-specific-libdir.c
@@ -1,8 +1,6 @@
 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the search path.
 //
-// REQUIRES: linux
-//
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
Index: test/Driver/arch-specific-libdir-rpath.c
===
--- test/Driver/arch-specific-libdir-rpath.c
+++ test/Driver/arch-specific-libdir-rpath.c
@@ -1,50 +1,53 @@
 // Test that the driver adds an arch-specific subdirectory in
-// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' for native
-// compilations.
-//
-// -rpath only gets added during native compilation.  To keep the test simple,
-// just test for x86_64-linux native compilation.
-// REQUIRES: x86_64-linux
+// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
 // Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
-// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN: -fsanitize=address -shared-libasan \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+//
+// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan on aarch64
+// RUN: %clang %s -### 2>&1 -target aarch64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-AArch64,RPATH-AArch64 %s
 //
 // Add LIBPATH, RPATH with -fsanitize=address for Android
 // RUN: %clang %s -### 2>&1 -target x86_64-linux-android -fsanitize=address \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH, RPATH for OpenMP
-// RUN: %clang %s -### 2>&1 -fopenmp \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fopenmp \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH but no RPATH for ubsan (or any other sanitizer)
 // RUN: %clang %s -### 2>&1 -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH but no RPATH if no sanitizer or runtime is specified
 // RUN: %clang %s -### 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Do not add LIBPATH or RPATH if arch-specific subdir doesn't exist
 // RUN: %clang %s -### 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-LIBPATH,NO-RPATH %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,NO-LIBPATH,NO-RPATH %s
 //
 //
-// FILEPATH: "-x" "c" "[[FILE_PATH:.*]]/{{.*}}.c"
-// LIBPATH: -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64
-// RPATH: "-rpath" "[[FILE_PATH]]/Inputs/resou

[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D30700#695358, @Hahnfeld wrote:

> No build system will ever set `-frtlib-add-rpath` to enable this "feature". 
> I'm for keeping this opt-out until we have configuration files to set this by 
> default. Making it opt-in would weaken its main reason of existence: Not to 
> break simple binaries for the user, and we can just drop it.


I don't agree. If we want to be good citizens on Linux, what's supposed to 
happen is that we install our shared libraries into /usr/lib/${distro_target}, 
which is what GCC does with its shared compiler runtime libraries. GCC does not 
add system-specific absolute rpaths to the binaries it produces.


https://reviews.llvm.org/D30700



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

[PATCH] D30700: [Driver] Always add arch-specific-subdir to -rpath

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama updated this revision to Diff 91635.
pirama added a comment.

*Actually* add the command line flags.


https://reviews.llvm.org/D30700

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/arch-specific-libdir-rpath.c
  test/Driver/arch-specific-libdir.c
  test/lit.cfg

Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -397,10 +397,6 @@
 if config.host_triple == config.target_triple:
 config.available_features.add("native")
 
-# Test Driver/arch-specific-libdir-rpath.c is restricted to x86_64-linux
-if re.match(r'^x86_64.*-linux', config.target_triple):
-config.available_features.add("x86_64-linux")
-
 # Case-insensitive file system
 def is_filesystem_case_insensitive():
 handle, path = tempfile.mkstemp(prefix='case-test', dir=config.test_exec_root)
Index: test/Driver/arch-specific-libdir.c
===
--- test/Driver/arch-specific-libdir.c
+++ test/Driver/arch-specific-libdir.c
@@ -1,8 +1,6 @@
 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the search path.
 //
-// REQUIRES: linux
-//
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
Index: test/Driver/arch-specific-libdir-rpath.c
===
--- test/Driver/arch-specific-libdir-rpath.c
+++ test/Driver/arch-specific-libdir-rpath.c
@@ -1,50 +1,85 @@
 // Test that the driver adds an arch-specific subdirectory in
-// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' for native
-// compilations.
+// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
-// -rpath only gets added during native compilation.  To keep the test simple,
-// just test for x86_64-linux native compilation.
-// REQUIRES: x86_64-linux
+// Test the default behavior when neither -frtlib-add-rpath nor
+// -fno-rtlib-add-rpath is specified, which is to skip -rpath
+// RUN: %clang %s -### 2>&1 -target x86_64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
+//
+// Test that -rpath is not added under -fno-rtlib-add-rpath even if other
+// conditions are met.
+// RUN: %clang %s -### 2>&1 -target x86_64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
+//
+// Test that -rpath is added only under the right circumstance even if
+// -frtlib-add-rpath is specified.
 //
 // Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
-// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
+//
+// Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN: -fsanitize=address -shared-libasan \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+//
+// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan on aarch64
+// RUN: %clang %s -### 2>&1 -target aarch64-linux \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-AArch64,RPATH-AArch64 %s
 //
 // Add LIBPATH, RPATH with -fsanitize=address for Android
 // RUN: %clang %s -### 2>&1 -target x86_64-linux-android -fsanitize=address \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH, RPATH for OpenMP
-// RUN: %clang %s -### 2>&1 -fopenmp \
+// RUN: %clang %s -

[PATCH] D30733: [Driver] Add arch-specific rpath for libc++

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama added inline comments.



Comment at: lib/Driver/ToolChain.cpp:652
+// libc++ may be installed per arch.
+addArchSpecificRPath(*this, Args, CmdArgs);
 break;

Hahnfeld wrote:
> pirama wrote:
> > `addArchSpecificRPath` is a static function in Tools.cpp and isn't visible 
> > here.
> No, it's not since recent refactoring. I do compile test my changes usually 
> ;-)
My bad.  I'd have found about it if I actually sync to ToT more often than once 
a week.


https://reviews.llvm.org/D30733



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

r297681 - Fix -Wunused-lambda-capture warning in new code

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Mar 13 17:33:07 2017
New Revision: 297681

URL: http://llvm.org/viewvc/llvm-project?rev=297681&view=rev
Log:
Fix -Wunused-lambda-capture warning in new code

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

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=297681&r1=297680&r2=297681&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Mar 13 17:33:07 2017
@@ -177,7 +177,7 @@ namespace clang {
   Err = Linker::linkModules(
   *getModule(), std::move(LM.Module), LM.LinkFlags,
   [](llvm::Module &M, const llvm::StringSet<> &GVS) {
-internalizeModule(M, [&M, &GVS](const llvm::GlobalValue &GV) {
+internalizeModule(M, [&GVS](const llvm::GlobalValue &GV) {
   return !GV.hasName() || (GVS.count(GV.getName()) == 0);
 });
   });


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