[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

2019-09-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr added a comment.

`PathDiagnosticConsumerOptions` is pretty lightweight, and is not passed around 
in hot paths if I understand correctly. What do you think about passing it by 
value everywhere?


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

https://reviews.llvm.org/D67420



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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

2019-09-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr added inline comments.



Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.def:23
+ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using 
HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF 
file", createSarifDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", 
createTextPathDiagnosticConsumer)

80 columns?



Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.h:36
 const Preprocessor &PP,
\
 const cross_tu::CrossTranslationUnitContext &CTU);
+#include "clang/Analysis/PathDiagnosticConsumers.def"

Maybe not quite an appropriate comment for this patch, but I just realized that 
this factory function returns void, which confused me -- where does the new 
consumer go? Only after reading an implementation of one of these functions I 
understood that they add the new consumer to `C`, which is a vector (whose type 
is conveniently hidden by a typedef -- why?)

I'd suggest to make these factories more like factories, and make them return a 
unique_ptr that the caller can put wherever they need. Of course, in a separate 
patch.

I'd also suggest to remove the `PathDiagnosticConsumers` typedef altogether. It 
is used just a couple of times in the code, and obfuscates code more than it 
helps.



Comment at: clang/lib/Analysis/PlistHTMLDiagnostics.cpp:27
+const cross_tu::CrossTranslationUnitContext &CTU) {
+  // Duplicate the DiagOpts object into both consumers.
+  createHTMLDiagnosticConsumer(PathDiagnosticConsumerOptions(DiagOpts), C,

Passing it by value would have made it less subtle, and probably as efficient.



Comment at: clang/lib/Analysis/TextDiagnostics.cpp:23
+namespace {
+class TextDiagnostics : public PathDiagnosticConsumer {
+  DiagnosticsEngine &Diag;

"TextDiagnosticsConsumer"

Or even better, "TextPathDiagnosticsConsumer".

Same for the file name. Same for other files that define diagnostics consumers.

Clarity is really important here. Very few places in the code will mention this 
type by name, however, it is really important to distinguish this diagnostics 
infrastructure from the rest of Clang's diagnostics.

I'm also starting to wonder whether this diagnostics infrastructure should be 
in its own library, not in libAnalysis -- what do you think?


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

https://reviews.llvm.org/D67422



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

> Hm, I was not aware of this Linux linker feature, thanks a lot for the 
> explanation! I see only one problem with using it as a replacement for the 
> begin/end objects – it looks like `__start_name`/`__stop_name` symbols are 
> created with `default` visibility instead of `hidden`. I guess it will cause 
> problems for offload programs that use shared libraries because DSO’s 
> `__start_name`/`__stop_name` symbols will be preempted by the executable’s 
> symbols and that is not what we want. Is there any way to change this 
> behavior?

Declaring the symbol as `__attribute__((__visibility__("hidden")))` just works 
as far as I can tell. The linker still provides the right definition, objdump 
says it's hidden.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

2019-09-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr added inline comments.



Comment at: clang/include/clang/Analysis/IssueHash.h:18
 
 /// Get an MD5 hash to help identify bugs.
 ///

Returns an opaque identifier for a diagnostic.

This opaque identifier is intended to be stable even when the source code is 
changed. It allows to track diagnostics in the long term, for example, find 
which diagnostics are "new", maintain a database of suppressed diagnostics etc.

The identifier includes the following information:
- the normalized source text...
- ...

The identifier does not include the following information:
- the name of the file,
- the line and column number in the file,
- ...



Comment at: clang/include/clang/Analysis/IssueHash.h:35
+///
 /// In case a new hash is introduced, the old one should still be maintained 
for
 /// a while. One should not introduce a new hash for every change, it is

I don't understand what this paragraph is talking about at all. What does it 
mean for a new hash to be introduced? As in, when this hashing algorithm 
changes?



Comment at: clang/include/clang/Analysis/IssueHash.h:41
+llvm::SmallString<32>
+GetIssueHash(FullSourceLoc &IssueLoc,
+ llvm::StringRef CheckerName, llvm::StringRef WarningMessage,

Why isn't `IssueLoc` const?



Comment at: clang/include/clang/Analysis/IssueHash.h:47
 /// more information.
-std::string GetIssueString(const SourceManager &SM, FullSourceLoc &IssueLoc,
-   llvm::StringRef CheckerName, llvm::StringRef 
BugType,
-   const Decl *D, const LangOptions &LangOpts);
+std::string GetIssueString(FullSourceLoc &IssueLoc,
+   llvm::StringRef CheckerName,

The doc comment suggests that this function returns a hash as a string, but 
that's not what it does. It returns some sort of identifier that is then hashed.


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

https://reviews.llvm.org/D67421



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

I'm on board with getting rid of the linker script. Gold's limited support for 
that seems conclusive.

I believe the current script does two things:
1/ takes a binary and embeds it in a section named 
.omp_offloading.amdgcn-amd-amdhsa
2/ provides start/end symbols for that section and for .omp_offloading.entries.

2/ is discussed above.
1/ can be implemented as a call to (llvm-)objcopy

> If binary is used as the value for --input-target, the input file will be 
> embedded as a data section in an ELF relocatable object, with symbols 
> _binary__start, _binary__end, and 
> _binary__size representing the start, end and size of the data, 
> where  is the path of the input file as specified on the command 
> line with non-alphanumeric characters converted to _.

I think dropping the linker script means that cmake will need to invoke an 
extra executable. As far as I can see, that tool can be objcopy instead of 
clang-offload-wrapper.

Does this diff mix getting rid of the linker script with other changes? E.g. it 
looks like the metadata generation is moving from clang to the new tool, but 
that seems orthogonal to dropping the linker script.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67452: [clang] [unittest] Import LLVMTestingSupport if necessary

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

Thanks for the fix! I'm not a fan of the complexity that standalone builds 
create, but if we are supporting them... oh well.


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

https://reviews.llvm.org/D67452



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

[PATCH] D56661: [clang-tidy] Fix incorrect array name generation in cppcoreguidelines-pro-bounds-constant-array-index

2019-09-12 Thread Jonas Toth via Phabricator via cfe-commits 
JonasToth added a comment.

Sorry for the long delay!

I have one question about a slighty different code constellation.

  std::optional> my_array;
  
  for(int i = 0; i < 42; ++i)
  (*my_array)[i] = 42;

Will this be fixed properly?

  for(int i = 0; i < 42; ++i)
  gsl::at(*my_array, i) = 42;

If the braces in `(*my_array)` would be moved into the function that is ok as 
well.

This is just of general interest and should not block this patch in my opinion, 
as its a different bug.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56661



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

r371719 - [clang-format] Add new style option IndentGotoLabels

2019-09-12 Thread Paul Hoad via cfe-commits 
Author: paulhoad
Date: Thu Sep 12 03:07:14 2019
New Revision: 371719

URL: http://llvm.org/viewvc/llvm-project?rev=371719&view=rev
Log:
[clang-format] Add new style option IndentGotoLabels

Summary:
This option determines whether goto labels are indented according to scope. 
Setting this option to false causes goto labels to be flushed to the left.
This is mostly copied from [[ 
http://lists.llvm.org/pipermail/cfe-dev/2015-September/045014.html | this patch 
]] submitted by Christian Neukirchen that didn't make its way into trunk.

```
 true:  false:
 int f() {  vs. int f() {
   if (foo()) {   if (foo()) {
   label1:  label1:
 bar(); bar();
   }  }
 label2:label2:
   return 1;  return 1;
 }  }
```

Reviewers: klimek, MyDeveloperDay

Reviewed By: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang, #clang-tools-extra

Patch by: tetsuo-cpp

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=371719&r1=371718&r2=371719&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Sep 12 03:07:14 2019
@@ -1527,6 +1527,23 @@ the configuration (without a prefix: ``A
plop();  plop();
  }  }
 
+**IndentGotoLabels** (``bool``)
+  Indent goto labels.
+
+  When ``false``, goto labels are flushed left.
+
+  .. code-block:: c++
+
+ true:  false:
+ int f() {  vs. int f() {
+   if (foo()) {   if (foo()) {
+   label1:  label1:
+ bar(); bar();
+   }  }
+ label2:label2:
+   return 1;  return 1;
+ }  }
+
 **IndentPPDirectives** (``PPDirectiveIndentStyle``)
   The preprocessor directive indenting style to use.
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=371719&r1=371718&r2=371719&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Thu Sep 12 03:07:14 2019
@@ -1265,6 +1265,22 @@ struct FormatStyle {
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent goto labels.
+  ///
+  /// When ``false``, goto labels are flushed left.
+  /// \code
+  ///true:  false:
+  ///int f() {  vs. int f() {
+  ///  if (foo()) {   if (foo()) {
+  ///  label1:  label1:
+  ///bar(); bar();
+  ///  }  }
+  ///label2:label2:
+  ///  return 1;  return 1;
+  ///}  }
+  /// \endcode
+  bool IndentGotoLabels;
+
   /// Options for indenting preprocessor directives.
   enum PPDirectiveIndentStyle {
 /// Does not indent any directives.
@@ -1990,6 +2006,7 @@ struct FormatStyle {
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories 
&&
IndentCaseLabels == R.IndentCaseLabels &&
+   IndentGotoLabels == R.IndentGotoLabels &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=371719&r1=371718&r2=371719&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Sep 12 03:07:14 2019
@@ -453,6 +453,7 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

2019-09-12 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371719: [clang-format] Add new style option IndentGotoLabels 
(authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67037?vs=219582&id=219871#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67037

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.h
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1527,6 +1527,23 @@
plop();  plop();
  }  }
 
+**IndentGotoLabels** (``bool``)
+  Indent goto labels.
+
+  When ``false``, goto labels are flushed left.
+
+  .. code-block:: c++
+
+ true:  false:
+ int f() {  vs. int f() {
+   if (foo()) {   if (foo()) {
+   label1:  label1:
+ bar(); bar();
+   }  }
+ label2:label2:
+   return 1;  return 1;
+ }  }
+
 **IndentPPDirectives** (``PPDirectiveIndentStyle``)
   The preprocessor directive indenting style to use.
 
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1265,6 +1265,22 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent goto labels.
+  ///
+  /// When ``false``, goto labels are flushed left.
+  /// \code
+  ///true:  false:
+  ///int f() {  vs. int f() {
+  ///  if (foo()) {   if (foo()) {
+  ///  label1:  label1:
+  ///bar(); bar();
+  ///  }  }
+  ///label2:label2:
+  ///  return 1;  return 1;
+  ///}  }
+  /// \endcode
+  bool IndentGotoLabels;
+
   /// Options for indenting preprocessor directives.
   enum PPDirectiveIndentStyle {
 /// Does not indent any directives.
@@ -1990,6 +2006,7 @@
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories &&
IndentCaseLabels == R.IndentCaseLabels &&
+   IndentGotoLabels == R.IndentGotoLabels &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
Index: cfe/trunk/lib/Format/UnwrappedLineParser.h
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.h
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(!Style.IndentGotoLabels);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (LeftAlignLabel)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.After

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

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

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932



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

r371720 - [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

2019-09-12 Thread Paul Hoad via cfe-commits 
Author: paulhoad
Date: Thu Sep 12 03:18:53 2019
New Revision: 371720

URL: http://llvm.org/viewvc/llvm-project?rev=371720&view=rev
Log:
[clang-format] [PR43100] clang-format C#  support does not add a space between 
"using" and paren

Summary:
Addresses https://bugs.llvm.org/show_bug.cgi?id=43100

Formatting using statement in C# with clang-format removes the space between 
using and paren even when SpaceBeforeParens is !

```
using(FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, 
FileShare.Read, bufferSize : 1))
```

this change simply overcomes this for when using C# settings in the 
.clang-format file

```
using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, 
FileShare.Read, bufferSize : 1))
```

All FormatTests pass..

```
[==] 688 tests from 21 test cases ran. (88508 ms total)
[  PASSED  ] 688 tests.
```

Reviewers: djasper, klimek, owenpan

Reviewed By: owenpan

Subscribers: llvm-commits, cfe-commits

Tags: #clang

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

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=371720&r1=371719&r2=371720&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Sep 12 03:18:53 2019
@@ -2611,6 +2611,10 @@ bool TokenAnnotator::spaceRequiredBetwee
 return Style.Language == FormatStyle::LK_JavaScript ||
!Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never)
+  return true;
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;

Modified: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestCSharp.cpp?rev=371720&r1=371719&r2=371720&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp Thu Sep 12 03:18:53 2019
@@ -165,6 +165,21 @@ TEST_F(FormatTestCSharp, Attributes) {
"public string Host {\n  set;\n  get;\n}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
+   "}",
+   Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("public void foo() {\n"
+   "  using(StreamWriter sw = new StreamWriter(filenameB)) {}\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaa a "
"aaa long region");


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

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

2019-09-12 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371720: [clang-format] [PR43100] clang-format C#  support 
does not add a space between… (authored by paulhoad, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D2?vs=217167&id=219873#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D2

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


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 return Style.Language == FormatStyle::LK_JavaScript ||
!Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never)
+  return true;
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"public string Host {\n  set;\n  get;\n}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
+   "}",
+   Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("public void foo() {\n"
+   "  using(StreamWriter sw = new StreamWriter(filenameB)) {}\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaa a "
"aaa long region");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2611,6 +2611,10 @@
 return Style.Language == FormatStyle::LK_JavaScript ||
!Left.TokenText.endswith("=*/");
   if (Right.is(tok::l_paren)) {
+// using (FileStream fs...
+if (Style.isCSharp() && Left.is(tok::kw_using) &&
+Style.SpaceBeforeParens != FormatStyle::SBPO_Never)
+  return true;
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
Index: cfe/trunk/unittests/Format/FormatTestCSharp.cpp
===
--- cfe/trunk/unittests/Format/FormatTestCSharp.cpp
+++ cfe/trunk/unittests/Format/FormatTestCSharp.cpp
@@ -165,6 +165,21 @@
"public string Host {\n  set;\n  get;\n}");
 }
 
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter (filenameA)) {}\n"
+   "}",
+   Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("public void foo() {\n"
+   "  using(StreamWriter sw = new StreamWriter(filenameB)) {}\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpRegions) {
   verifyFormat("#region aaa a "
"aaa long region");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r371663 - Start porting ivfsoverlay tests to Windows

2019-09-12 Thread Jeremy Morse via cfe-commits 
Hi Reid,

On Wed, Sep 11, 2019 at 9:55 PM Reid Kleckner via cfe-commits
 wrote:
> +// XFAIL: windows

Looks like this should be system-windows, the PS4 buildbots [0] have a
windows host but target x86_64-scei-ps4, and croak on this change.

Assuming no-one else is looking at this, I'll commit a fix in ~30 mins.

[0] 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/28114

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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

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

Can you add a test for `riscv64-unknown-linux-gnu`? I think RISCVToolchain.cpp 
is only used by the `riscv64-unknown-elf` target, but I could be wrong.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67409



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

r371723 - Removed dead code from DiagnosticBuilder

2019-09-12 Thread Dmitri Gribenko via cfe-commits 
Author: gribozavr
Date: Thu Sep 12 03:39:53 2019
New Revision: 371723

URL: http://llvm.org/viewvc/llvm-project?rev=371723&view=rev
Log:
Removed dead code from DiagnosticBuilder

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

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=371723&r1=371722&r2=371723&view=diff
==
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Thu Sep 12 03:39:53 2019
@@ -1127,11 +1127,6 @@ public:
 Emit();
   }
 
-  /// Retrieve an empty diagnostic builder.
-  static DiagnosticBuilder getEmpty() {
-return {};
-  }
-
   /// Forces the diagnostic to be emitted.
   const DiagnosticBuilder &setForceEmit() const {
 IsForceEmit = true;


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

[PATCH] D67490: [Clang][ASTImporter] Added visibility check for FunctionTemplateDecl.

2019-09-12 Thread Balázs Kéri via Phabricator via cfe-commits 
balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

ASTImporter makes now difference between function templates with same
name in different translation units if these are not visible outside.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67490

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterVisibilityTest.cpp

Index: clang/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- clang/unittests/AST/ASTImporterVisibilityTest.cpp
+++ clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -43,6 +43,12 @@
   using DeclTy = TypedefNameDecl;
   BindableMatcher operator()() { return typedefNameDecl(hasName("T")); }
 };
+struct GetFunTemplPattern {
+  using DeclTy = FunctionTemplateDecl;
+  BindableMatcher operator()() {
+return functionTemplateDecl(hasName("f"));
+  }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -64,6 +70,10 @@
 const auto *AnonTypedef = "namespace { typedef int T; }";
 const auto *ExternUsing = "using T = int;";
 const auto *AnonUsing = "namespace { using T = int; }";
+// FunctionTemplateDecl:
+const auto *ExternFT = "template  void f();";
+const auto *StaticFT = "template  static void f();";
+const auto *AnonFT = "namespace { template  void f(); }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -108,6 +118,9 @@
 using ImportFunctionsVisibilityChain = ImportVisibilityChain;
 using ImportVariablesVisibilityChain = ImportVisibilityChain;
 using ImportClassesVisibilityChain = ImportVisibilityChain;
+using ImportFunctionTemplatesVisibilityChain =
+ImportVisibilityChain;
+
 // Value-parameterized test for functions.
 TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
@@ -120,6 +133,10 @@
 TEST_P(ImportClassesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for function templates.
+TEST_P(ImportFunctionTemplatesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 
 // Automatic instantiation of the value-parameterized tests.
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
@@ -143,6 +160,11 @@
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
 ::testing::Values(ExternC, AnonC)), );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests,
+ImportFunctionTemplatesVisibilityChain,
+::testing::Combine(DefaultTestValuesForRunOptions,
+   ::testing::Values(ExternFT, StaticFT,
+ AnonFT)), );
 
 // First value in tuple: Compile options.
 // Second value in tuple: Tuple with informations for the test.
@@ -253,6 +275,7 @@
 using ImportClassesVisibility = ImportVisibility;
 using ImportEnumsVisibility = ImportVisibility;
 using ImportTypedefNameVisibility = ImportVisibility;
+using ImportFunctionTemplatesVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -289,6 +312,13 @@
 TEST_P(ImportTypedefNameVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImportWithMerge();
 }
+// FunctionTemplateDecl.
+TEST_P(ImportFunctionTemplatesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportFunctionTemplatesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 
 const bool ExpectLinkedDeclChain = true;
 const bool ExpectUnlinkedDeclChain = false;
@@ -367,6 +397,20 @@
 std::make_tuple(AnonTypedef, ExternUsing, ExpectUnlinkedDeclChain),
 std::make_tuple(AnonTypedef, AnonUsing,
 ExpectUnlinkedDeclChain))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportFunctionTemplatesVisibility,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(
+std::make_tuple(ExternFT, ExternFT, ExpectLinkedDeclChain),
+std::make_tuple(ExternFT, StaticFT, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternFT, AnonFT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticFT, ExternFT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticFT, StaticFT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticFT, AnonFT, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonFT, ExternFT, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonFT, StaticFT, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonFT, AnonFT, ExpectUnlinkedDeclChain))), );
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporte

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

2019-09-12 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon added a comment.

Thanks - this builds cleanly on MSVC now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

2019-09-12 Thread Nandor Licker via Phabricator via cfe-commits 
nand added a comment.

Thanks for looking into the problem - sorry for the delay!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

r371726 - Switch "windows" to "system-windows" in some XFAILs

2019-09-12 Thread Jeremy Morse via cfe-commits 
Author: jmorse
Date: Thu Sep 12 04:19:12 2019
New Revision: 371726

URL: http://llvm.org/viewvc/llvm-project?rev=371726&view=rev
Log:
Switch "windows" to "system-windows" in some XFAILs

The test failure mode appears to be due to the host machine rather than the
target. The PS4 buildbots are windows-hosted targeting x86_64-scei-ps4,
and are currently reporting these as unexpected failures:

  
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/28114

Modified:
cfe/trunk/test/Index/index-module-with-vfs.m
cfe/trunk/test/Modules/double-quotes.m
cfe/trunk/test/Modules/framework-public-includes-private.m
cfe/trunk/test/VFS/external-names.c
cfe/trunk/test/VFS/framework-import.m
cfe/trunk/test/VFS/implicit-include.c
cfe/trunk/test/VFS/include-mixed-real-and-virtual.c
cfe/trunk/test/VFS/include-real-from-virtual.c
cfe/trunk/test/VFS/include-virtual-from-real.c
cfe/trunk/test/VFS/include.c
cfe/trunk/test/VFS/incomplete-umbrella.m
cfe/trunk/test/VFS/module-import.m
cfe/trunk/test/VFS/real-path-found-first.m
cfe/trunk/test/VFS/relative-path.c
cfe/trunk/test/VFS/subframework-symlink.m
cfe/trunk/test/VFS/umbrella-framework-import-skipnonexist.m
cfe/trunk/test/VFS/vfsroot-include.c
cfe/trunk/test/VFS/vfsroot-module.m
cfe/trunk/test/VFS/vfsroot-with-overlay.c

Modified: cfe/trunk/test/Index/index-module-with-vfs.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-module-with-vfs.m?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/Index/index-module-with-vfs.m (original)
+++ cfe/trunk/test/Index/index-module-with-vfs.m Thu Sep 12 04:19:12 2019
@@ -1,5 +1,5 @@
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 @import ModuleNeedsVFS;
 

Modified: cfe/trunk/test/Modules/double-quotes.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/double-quotes.m?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/Modules/double-quotes.m (original)
+++ cfe/trunk/test/Modules/double-quotes.m Thu Sep 12 04:19:12 2019
@@ -1,5 +1,5 @@
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 // RUN: rm -rf %t
 // RUN: mkdir %t

Modified: cfe/trunk/test/Modules/framework-public-includes-private.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/framework-public-includes-private.m?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/Modules/framework-public-includes-private.m (original)
+++ cfe/trunk/test/Modules/framework-public-includes-private.m Thu Sep 12 
04:19:12 2019
@@ -1,5 +1,5 @@
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 // RUN: rm -rf %t
 // RUN: mkdir %t

Modified: cfe/trunk/test/VFS/external-names.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/external-names.c?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/VFS/external-names.c (original)
+++ cfe/trunk/test/VFS/external-names.c Thu Sep 12 04:19:12 2019
@@ -2,7 +2,7 @@
 // RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" -e 
"s@EXTERNAL_NAMES@false@" %S/Inputs/use-external-names.yaml > %t.yaml
 
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 #include "external-names.h"
 #ifdef REINCLUDE

Modified: cfe/trunk/test/VFS/framework-import.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/framework-import.m?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/VFS/framework-import.m (original)
+++ cfe/trunk/test/VFS/framework-import.m Thu Sep 12 04:19:12 2019
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -Werror -F %t -ivfsoverlay %t.yaml -fsyntax-only %s
 
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 #import 
 

Modified: cfe/trunk/test/VFS/implicit-include.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/implicit-include.c?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/VFS/implicit-include.c (original)
+++ cfe/trunk/test/VFS/implicit-include.c Thu Sep 12 04:19:12 2019
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %t -include "not_real.h" 
-fsyntax-only %s
 
 // FIXME: PR43272
-// XFAIL: windows
+// XFAIL: system-windows
 
 void foo() {
   bar();

Modified: cfe/trunk/test/VFS/include-mixed-real-and-virtual.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/include-mixed-real-and-virtual.c?rev=371726&r1=371725&r2=371726&view=diff
==
--- cfe/trunk/test/VFS/include-

[PATCH] D67491: Removed some questionable default arguments from setters

2019-09-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

They can be confusing -- what does it mean to call a setter without a
value? Also, some setters, like `setPrintTemplateTree` had `false` as
the default value!

The callers are largely not using these default arguments anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67491

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/Sema.cpp

Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1307,7 +1307,7 @@
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
   return;
 
@@ -1332,7 +1332,7 @@
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
 
   // Now the diagnostic state is clear, produce a C++98 compatibility
@@ -1341,7 +1341,7 @@
 
   // The last diagnostic which Sema produced was ignored. Suppress any
   // notes attached to it.
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   return;
 }
 
@@ -1355,7 +1355,7 @@
   }
 
   // Suppress this diagnostic.
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
   return;
 }
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -928,7 +928,7 @@
 // 'expected' comments.
 if (CI.getDiagnosticOpts().VerifyDiagnostics) {
   // Make sure we don't emit new diagnostics!
-  CI.getDiagnostics().setSuppressAllDiagnostics();
+  CI.getDiagnostics().setSuppressAllDiagnostics(true);
   Preprocessor &PP = getCompilerInstance().getPreprocessor();
   PP.EnterMainSourceFile();
   Token Tok;
Index: clang/lib/ARCMigrate/ARCMT.cpp
===
--- clang/lib/ARCMigrate/ARCMT.cpp
+++ clang/lib/ARCMigrate/ARCMT.cpp
@@ -139,7 +139,7 @@
 }
 
 // Non-ARC warnings are ignored.
-Diags.setLastDiagnosticIgnored();
+Diags.setLastDiagnosticIgnored(true);
   }
 };
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -632,24 +632,22 @@
   /// Suppress all diagnostics, to silence the front end when we
   /// know that we don't want any more diagnostics to be passed along to the
   /// client
-  void setSuppressAllDiagnostics(bool Val = true) {
-SuppressAllDiagnostics = Val;
-  }
+  void setSuppressAllDiagnostics(bool Val) { SuppressAllDiagnostics = Val; }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
-  void setElideType(bool Val = true) { ElideType = Val; }
+  void setElideType(bool Val) { ElideType = Val; }
   bool getElideType() { return ElideType; }
 
   /// Set tree printing, to outputting the template difference in a
   /// tree format.
-  void setPrintTemplateTree(bool Val = false) { PrintTemplateTree = Val; }
+  void setPrintTemplateTree(bool Val) { PrintTemplateTree = Val; }
   bool getPrintTemplateTree() { return PrintTemplateTree; }
 
   /// Set color printing, so the type diffing will inject color markers
   /// into the output.
-  void setShowColors(bool Val = false) { ShowColors = Val; }
+  void setShowColors(bool Val) { ShowColors = Val; }
   bool getShowColors() { return ShowColors; }
 
   /// Specify which overload candidates to show when overload resolution
@@ -667,7 +665,7 @@
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored = true) {
+  void setLastDiagnosticIgnored(bool Ignored) {
 if (LastDiagLevel == DiagnosticIDs::Fatal)
   FatalErrorOccurred = true;
 LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r371663 - Start porting ivfsoverlay tests to Windows

2019-09-12 Thread Jeremy Morse via cfe-commits 
On Thu, Sep 12, 2019 at 11:24 AM Jeremy Morse
 wrote:
> Assuming no-one else is looking at this, I'll commit a fix in ~30 mins.

r371726

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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Unfortunately, only considering parenthesis (`l_paren`, `r_paren`) is not 
sufficient. We need to consider all the nesting tokens, including brackets 
(`l_square`, `r_square`) and braces (`l_brace`, `r_brace`), since the Standard 
says (C++ 17 Draft N4659, Section 17.2/3 
):

> [...] When parsing a template-argument-list, the first non-nested > is taken 
> as the ending delimiter rather than a greater-than operator. [...]

Example with brackets that fails with your updated patch:

  template 
  struct S {};
  
  constexpr bool b[1] = { true };
  
  typedef S S_t, *S_p;

The following is an example with braces inside a template argument:

  struct T {
constexpr T(bool) {}

static constexpr bool b = true;  
  };
  
  typedef S S_t, *S_p;

Note though, that the current code aborts upon encountering braces to avoid 
removing a `typedef struct {...} T;` case and therefore, isn't even able to 
handle a single declaration chain with braces in a template argument, such as 
`typedef S S_t;`.

As to handling the comma cases:
(A) It is certainly not straightforward, since typedef declaration chains with 
multiple declarations are internally split up into separate declarations and 
the checker is called for each of these declarations individually.
(B) Your expansion would be valid, however, I think it might be easier to 
implement the expansion to

  using S_t = S<(0 < 0)>;
  using S_p = S<(0 < 0)>*;


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

r371731 - Removed some questionable default arguments from setters

2019-09-12 Thread Dmitri Gribenko via cfe-commits 
Author: gribozavr
Date: Thu Sep 12 05:16:43 2019
New Revision: 371731

URL: http://llvm.org/viewvc/llvm-project?rev=371731&view=rev
Log:
Removed some questionable default arguments from setters

Summary:
They can be confusing -- what does it mean to call a setter without a
value? Also, some setters, like `setPrintTemplateTree` had `false` as
the default value!

The callers are largely not using these default arguments anyway.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/lib/ARCMigrate/ARCMT.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Sema/Sema.cpp

Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=371731&r1=371730&r2=371731&view=diff
==
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Thu Sep 12 05:16:43 2019
@@ -632,24 +632,22 @@ public:
   /// Suppress all diagnostics, to silence the front end when we
   /// know that we don't want any more diagnostics to be passed along to the
   /// client
-  void setSuppressAllDiagnostics(bool Val = true) {
-SuppressAllDiagnostics = Val;
-  }
+  void setSuppressAllDiagnostics(bool Val) { SuppressAllDiagnostics = Val; }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
-  void setElideType(bool Val = true) { ElideType = Val; }
+  void setElideType(bool Val) { ElideType = Val; }
   bool getElideType() { return ElideType; }
 
   /// Set tree printing, to outputting the template difference in a
   /// tree format.
-  void setPrintTemplateTree(bool Val = false) { PrintTemplateTree = Val; }
+  void setPrintTemplateTree(bool Val) { PrintTemplateTree = Val; }
   bool getPrintTemplateTree() { return PrintTemplateTree; }
 
   /// Set color printing, so the type diffing will inject color markers
   /// into the output.
-  void setShowColors(bool Val = false) { ShowColors = Val; }
+  void setShowColors(bool Val) { ShowColors = Val; }
   bool getShowColors() { return ShowColors; }
 
   /// Specify which overload candidates to show when overload resolution
@@ -667,7 +665,7 @@ public:
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored = true) {
+  void setLastDiagnosticIgnored(bool Ignored) {
 if (LastDiagLevel == DiagnosticIDs::Fatal)
   FatalErrorOccurred = true;
 LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;

Modified: cfe/trunk/lib/ARCMigrate/ARCMT.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ARCMT.cpp?rev=371731&r1=371730&r2=371731&view=diff
==
--- cfe/trunk/lib/ARCMigrate/ARCMT.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/ARCMT.cpp Thu Sep 12 05:16:43 2019
@@ -139,7 +139,7 @@ public:
 }
 
 // Non-ARC warnings are ignored.
-Diags.setLastDiagnosticIgnored();
+Diags.setLastDiagnosticIgnored(true);
   }
 };
 

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=371731&r1=371730&r2=371731&view=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Thu Sep 12 05:16:43 2019
@@ -928,7 +928,7 @@ void PrintDependencyDirectivesSourceMini
 // 'expected' comments.
 if (CI.getDiagnosticOpts().VerifyDiagnostics) {
   // Make sure we don't emit new diagnostics!
-  CI.getDiagnostics().setSuppressAllDiagnostics();
+  CI.getDiagnostics().setSuppressAllDiagnostics(true);
   Preprocessor &PP = getCompilerInstance().getPreprocessor();
   PP.EnterMainSourceFile();
   Token Tok;

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=371731&r1=371730&r2=371731&view=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Thu Sep 12 05:16:43 2019
@@ -1307,7 +1307,7 @@ void Sema::EmitCurrentDiagnostic(unsigne
PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
   return;
 
@@ -1332,7 +1332,7 @@ void Sema::EmitCurrentDiagnostic(unsigne
PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDia

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Also note that your enhanced version of my first example actually works with 
your initial patch, since the two comparison operators cancel each other out in 
the counting logic.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D67491: Removed some questionable default arguments from setters

2019-09-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371731: Removed some questionable default arguments from 
setters (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67491?vs=219882&id=219892#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67491

Files:
  cfe/trunk/include/clang/Basic/Diagnostic.h
  cfe/trunk/lib/ARCMigrate/ARCMT.cpp
  cfe/trunk/lib/Frontend/FrontendActions.cpp
  cfe/trunk/lib/Sema/Sema.cpp

Index: cfe/trunk/include/clang/Basic/Diagnostic.h
===
--- cfe/trunk/include/clang/Basic/Diagnostic.h
+++ cfe/trunk/include/clang/Basic/Diagnostic.h
@@ -632,24 +632,22 @@
   /// Suppress all diagnostics, to silence the front end when we
   /// know that we don't want any more diagnostics to be passed along to the
   /// client
-  void setSuppressAllDiagnostics(bool Val = true) {
-SuppressAllDiagnostics = Val;
-  }
+  void setSuppressAllDiagnostics(bool Val) { SuppressAllDiagnostics = Val; }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
-  void setElideType(bool Val = true) { ElideType = Val; }
+  void setElideType(bool Val) { ElideType = Val; }
   bool getElideType() { return ElideType; }
 
   /// Set tree printing, to outputting the template difference in a
   /// tree format.
-  void setPrintTemplateTree(bool Val = false) { PrintTemplateTree = Val; }
+  void setPrintTemplateTree(bool Val) { PrintTemplateTree = Val; }
   bool getPrintTemplateTree() { return PrintTemplateTree; }
 
   /// Set color printing, so the type diffing will inject color markers
   /// into the output.
-  void setShowColors(bool Val = false) { ShowColors = Val; }
+  void setShowColors(bool Val) { ShowColors = Val; }
   bool getShowColors() { return ShowColors; }
 
   /// Specify which overload candidates to show when overload resolution
@@ -667,7 +665,7 @@
   /// the middle of another diagnostic.
   ///
   /// This can be used by clients who suppress diagnostics themselves.
-  void setLastDiagnosticIgnored(bool Ignored = true) {
+  void setLastDiagnosticIgnored(bool Ignored) {
 if (LastDiagLevel == DiagnosticIDs::Fatal)
   FatalErrorOccurred = true;
 LastDiagLevel = Ignored ? DiagnosticIDs::Ignored : DiagnosticIDs::Warning;
Index: cfe/trunk/lib/Frontend/FrontendActions.cpp
===
--- cfe/trunk/lib/Frontend/FrontendActions.cpp
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp
@@ -928,7 +928,7 @@
 // 'expected' comments.
 if (CI.getDiagnosticOpts().VerifyDiagnostics) {
   // Make sure we don't emit new diagnostics!
-  CI.getDiagnostics().setSuppressAllDiagnostics();
+  CI.getDiagnostics().setSuppressAllDiagnostics(true);
   Preprocessor &PP = getCompilerInstance().getPreprocessor();
   PP.EnterMainSourceFile();
   Token Tok;
Index: cfe/trunk/lib/ARCMigrate/ARCMT.cpp
===
--- cfe/trunk/lib/ARCMigrate/ARCMT.cpp
+++ cfe/trunk/lib/ARCMigrate/ARCMT.cpp
@@ -139,7 +139,7 @@
 }
 
 // Non-ARC warnings are ignored.
-Diags.setLastDiagnosticIgnored();
+Diags.setLastDiagnosticIgnored(true);
   }
 };
 
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -1307,7 +1307,7 @@
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
   return;
 
@@ -1332,7 +1332,7 @@
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
   }
 
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
 
   // Now the diagnostic state is clear, produce a C++98 compatibility
@@ -1341,7 +1341,7 @@
 
   // The last diagnostic which Sema produced was ignored. Suppress any
   // notes attached to it.
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   return;
 }
 
@@ -1355,7 +1355,7 @@
   }
 
   // Suppress this diagnostic.
-  Diags.setLastDiagnosticIgnored();
+  Diags.setLastDiagnosticIgnored(true);
   Diags.Clear();
   return;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219891.
kousikk added a comment.

- Add validation inside the createFile() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -176,6 +176,8 @@
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
+  if (Entry->isDirectory())
+return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   return std::make_unique(
   llvm::MemoryBuffer::getMemBuffer(*Contents, Entry->getName(),
/*RequiresNullTerminator=*/false),
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -176,6 +176,8 @@
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
+  if (Entry->isDirectory())
+return llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   return std::make_unique(
   llvm::MemoryBuffer::getMemBuffer(*Contents, Entry->getName(),
/*RequiresNullTerminator=*/false),
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219890.
kousikk added a comment.

- Add validation inside the createFile() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -176,6 +176,8 @@
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
+  if (Entry->isDirectory())
+return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   return std::make_unique(
   llvm::MemoryBuffer::getMemBuffer(*Contents, Entry->getName(),
/*RequiresNullTerminator=*/false),
@@ -191,7 +193,7 @@
 
   // Check the local cache first.
   if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-return createFile(Entry);
+createFile(Entry);
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -176,6 +176,8 @@
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
+  if (Entry->isDirectory())
+return llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   return std::make_unique(
   llvm::MemoryBuffer::getMemBuffer(*Contents, Entry->getName(),
/*RequiresNullTerminator=*/false),
@@ -191,7 +193,7 @@
 
   // Check the local cache first.
   if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-return createFile(Entry);
+createFile(Entry);
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67066: [RISCV] Add option aliases: -mcmodel=medany and -mcmodel=medlow

2019-09-12 Thread Sam Elliott via Phabricator via cfe-commits 
lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.

Looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67066



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

r371733 - [clang] [unittest] Import LLVMTestingSupport if necessary

2019-09-12 Thread Michal Gorny via cfe-commits 
Author: mgorny
Date: Thu Sep 12 06:06:12 2019
New Revision: 371733

URL: http://llvm.org/viewvc/llvm-project?rev=371733&view=rev
Log:
[clang] [unittest] Import LLVMTestingSupport if necessary

Add LLVMTestingSupport directory from LLVM_MAIN_SRC_DIR when building
clang stand-alone and LLVMTestingSupport library is not present.  This
is needed to fix stand-alone builds without clang-tools-extra.

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

Modified:
cfe/trunk/unittests/CMakeLists.txt

Modified: cfe/trunk/unittests/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/CMakeLists.txt?rev=371733&r1=371732&r2=371733&view=diff
==
--- cfe/trunk/unittests/CMakeLists.txt (original)
+++ cfe/trunk/unittests/CMakeLists.txt Thu Sep 12 06:06:12 2019
@@ -1,6 +1,15 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if(CLANG_BUILT_STANDALONE)
+  # LLVMTestingSupport library is needed for some of the unittests.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang


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

[PATCH] D67452: [clang] [unittest] Import LLVMTestingSupport if necessary

2019-09-12 Thread Michał Górny via Phabricator via cfe-commits 
mgorny added a comment.

Thanks. Could you also ack the backport in 
https://bugs.llvm.org/show_bug.cgi?id=43281?


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

https://reviews.llvm.org/D67452



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

[PATCH] D67452: [clang] [unittest] Import LLVMTestingSupport if necessary

2019-09-12 Thread Michał Górny via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371733: [clang] [unittest] Import LLVMTestingSupport if 
necessary (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67452?vs=219736&id=219897#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67452

Files:
  cfe/trunk/unittests/CMakeLists.txt


Index: cfe/trunk/unittests/CMakeLists.txt
===
--- cfe/trunk/unittests/CMakeLists.txt
+++ cfe/trunk/unittests/CMakeLists.txt
@@ -1,6 +1,15 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if(CLANG_BUILT_STANDALONE)
+  # LLVMTestingSupport library is needed for some of the unittests.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang


Index: cfe/trunk/unittests/CMakeLists.txt
===
--- cfe/trunk/unittests/CMakeLists.txt
+++ cfe/trunk/unittests/CMakeLists.txt
@@ -1,6 +1,15 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if(CLANG_BUILT_STANDALONE)
+  # LLVMTestingSupport library is needed for some of the unittests.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67496: [clangd] Collect macros in the preamble region of the main file

2019-09-12 Thread Haojian Wu via Phabricator via cfe-commits 
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay.
Herald added a project: clang.

- store all macro references in the ParsedAST, which also helps to implement 
find references for macros.
- unify the two variants of CollectMainFileMacros.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67496

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Macro.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -425,8 +425,8 @@
   // Tokens that share a source range but have conflicting Kinds are not
   // highlighted.
   R"cpp(
-  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
-  #define DEF_CLASS(T) class T {};
+  #define $Macro[[DEF_MULTIPLE]](X) namespace X { class X { int X; }; }
+  #define $Macro[[DEF_CLASS]](T) class T {};
   // Preamble ends.
   $Macro[[DEF_MULTIPLE]](XYZ);
   $Macro[[DEF_MULTIPLE]](XYZW);
@@ -465,8 +465,8 @@
   }
 )cpp",
   R"cpp(
-  #define fail(expr) expr
-  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  #define $Macro[[fail]](expr) expr
+  #define $Macro[[assert]](COND) if (!(COND)) { fail("assertion failed" #COND); }
   // Preamble ends.
   $Primitive[[int]] $Variable[[x]];
   $Primitive[[int]] $Variable[[y]];
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -229,8 +229,8 @@
 
 TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   Annotations TestCase(R"cpp(
-#define MACRO_ARGS(X, Y) X Y
-// - premable ends, macros inside preamble are not considered in main file.
+#define ^MACRO_ARGS(X, Y) X Y
+// - preamble ends
 ^ID(int A);
 // Macro arguments included.
 ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2));
@@ -270,12 +270,12 @@
 int D = DEF;
   )cpp";
   ParsedAST AST = TU.build();
-  const std::vector &MacroExpansionLocations = AST.getMacros();
+  auto MacroExpansionLocations = AST.getMacros();
   std::vector MacroExpansionPositions;
   for (const auto &L : MacroExpansionLocations)
-MacroExpansionPositions.push_back(
-sourceLocToPosition(AST.getSourceManager(), L));
-  EXPECT_EQ(MacroExpansionPositions, TestCase.points());
+MacroExpansionPositions.push_back(L.R.start);
+  EXPECT_THAT(MacroExpansionPositions,
+  testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
 } // namespace
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,8 +38,9 @@
 TraverseAST(AST.getASTContext());
 // Add highlightings for macro expansions as they are not traversed by the
 // visitor.
-for (SourceLocation Loc : AST.getMacros())
-  addToken(Loc, HighlightingKind::Macro);
+for (const auto &M : AST.getMacros())
+  Tokens.push_back({HighlightingKind::Macro, M.R});
+// addToken(Loc, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -26,6 +26,7 @@
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Headers.h"
+#include "Macro.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -42,12 +43,6 @@
 /// As we must avoid re-parsing the preamble, any information that can only
 /// be obtained during parsing must be eagerly captured and stored here.
 struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
-   IncludeStructure Includes,
-   std::vector MainFileMacros,
-   std::unique_ptr StatCache,
-   CanonicalIncludes CanonIncludes);
-
   tooling::CompileCommand CompileCommand;
   PrecompiledPreamble Preamble;
   std::vector Diags;
@@ -57,11 +52,17 @@
   // Macros def

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

2019-09-12 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia updated this revision to Diff 219903.
Anastasia added a comment.

- Move addr space deduction to a later phase.


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

https://reviews.llvm.org/D65744

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaOpenCL/event_t.cl
  clang/test/SemaOpenCL/invalid-block.cl
  clang/test/SemaOpenCL/invalid-pipes-cl2.0.cl
  clang/test/SemaOpenCL/sampler_t.cl
  clang/test/SemaOpenCLCXX/address-space-deduction.cl
  clang/test/SemaOpenCLCXX/addrspace-auto.cl
  clang/test/SemaOpenCLCXX/restricted.cl

Index: clang/test/SemaOpenCLCXX/restricted.cl
===
--- clang/test/SemaOpenCLCXX/restricted.cl
+++ clang/test/SemaOpenCLCXX/restricted.cl
@@ -32,12 +32,14 @@
 __constant _Thread_local int a = 1;
 // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the '_Thread_local' storage class specifier}}
 // expected-warning@-2 {{'_Thread_local' is a C11 extension}}
-
+// expected-error@-3 {{thread-local storage is not supported for the current target}}
 __constant __thread int b = 2;
 // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the '__thread' storage class specifier}}
+// expected-error@-2 {{thread-local storage is not supported for the current target}}
 kernel void test_storage_classes() {
   register int x;
-  // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'register' storage class specifier}}
+// expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'register' storage class specifier}}
   thread_local int y;
-  // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'thread_local' storage class specifier}}
+// expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'thread_local' storage class specifier}}
+// expected-error@-2 {{thread-local storage is not supported for the current target}}
 }
Index: clang/test/SemaOpenCLCXX/addrspace-auto.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/addrspace-auto.cl
@@ -0,0 +1,35 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+__constant int i = 1;
+//CHECK: |-VarDecl {{.*}} ai '__global int':'__global int'
+auto ai = i;
+
+kernel void test() {
+  int i;
+  //CHECK: VarDecl {{.*}} ai 'int':'int'
+  auto ai = i;
+
+  constexpr int c = 1;
+  //CHECK: VarDecl {{.*}} used cai '__constant int':'__constant int'
+  __constant auto cai = c;
+  //CHECK: VarDecl {{.*}} aii 'int':'int'
+  auto aii = cai;
+
+  //CHECK: VarDecl {{.*}} ref 'int &'
+  auto &ref = i;
+  //CHECK: VarDecl {{.*}} ptr 'int *'
+  auto *ptr = &i;
+  //CHECK: VarDecl {{.*}} ref_c '__constant int &'
+  auto &ref_c = cai;
+
+  //CHECK: VarDecl {{.*}} ptrptr 'int *__generic *'
+  auto **ptrptr = &ptr;
+  //CHECK: VarDecl {{.*}} refptr 'int *__generic &'
+  auto *&refptr = ptr;
+
+  //CHECK: VarDecl {{.*}} invalid gref '__global auto &'
+  __global auto &gref = i; //expected-error{{variable 'gref' with type '__global auto &' has incompatible initializer of type 'int'}}
+  __local int *ptr_l;
+  //CHECK: VarDecl {{.*}} invalid gptr '__global auto *'
+  __global auto *gptr = ptr_l; //expected-error{{variable 'gptr' with type '__global auto *' has incompatible initializer of type '__local int *'}}
+}
Index: clang/test/SemaOpenCLCXX/address-space-deduction.cl
===
--- clang/test/SemaOpenCLCXX/address-space-deduction.cl
+++ clang/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -65,18 +65,30 @@
 x3::x3(const x3 &t) {}
 
 template 
-T xxx(T *in) {
+T xxx(T *in1, T in2) {
   // This pointer can't be deduced to generic because addr space
   // will be taken from the template argument.
   //CHECK: `-VarDecl {{.*}} i 'T *' cinit
-  T *i = in;
+  T *i = in1;
   T ii;
+  __private T *ptr = ⅈ
+  ptr = &in2;
   return *i;
 }
 
 __kernel void test() {
   int foo[10];
-  xxx(&foo[0]);
+  xxx<__private int>(&foo[0], foo[0]);
+  // FIXME: Template param deduction fails here because
+  // temporaries are not in the __private address space.
+  // It is probably reasonable to put them in __private
+  // considering that stack and function params are
+  // implicitly in __private.
+  // However, if temporaries are left in default addr
+  // space we should at least pretty print the __private
+  // addr space. Otherwise diagnostic apprears to be
+  // confusing.
+  //xxx(&foo[0], foo[0]);
 }
 
 // Addr space for pointer/reference to an array
Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

2019-09-12 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

Does this need landing? given that you have a number of patches in flight 
perhaps it would be good to request commit access


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D67496: [clangd] Collect macros in the preamble region of the main file

2019-09-12 Thread Haojian Wu via Phabricator via cfe-commits 
hokein updated this revision to Diff 219904.
hokein added a comment.

some cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67496

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Macro.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -425,8 +425,8 @@
   // Tokens that share a source range but have conflicting Kinds are not
   // highlighted.
   R"cpp(
-  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
-  #define DEF_CLASS(T) class T {};
+  #define $Macro[[DEF_MULTIPLE]](X) namespace X { class X { int X; }; }
+  #define $Macro[[DEF_CLASS]](T) class T {};
   // Preamble ends.
   $Macro[[DEF_MULTIPLE]](XYZ);
   $Macro[[DEF_MULTIPLE]](XYZW);
@@ -465,8 +465,8 @@
   }
 )cpp",
   R"cpp(
-  #define fail(expr) expr
-  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  #define $Macro[[fail]](expr) expr
+  #define $Macro[[assert]](COND) if (!(COND)) { fail("assertion failed" #COND); }
   // Preamble ends.
   $Primitive[[int]] $Variable[[x]];
   $Primitive[[int]] $Variable[[y]];
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -229,8 +229,8 @@
 
 TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   Annotations TestCase(R"cpp(
-#define MACRO_ARGS(X, Y) X Y
-// - premable ends, macros inside preamble are not considered in main file.
+#define ^MACRO_ARGS(X, Y) X Y
+// - preamble ends
 ^ID(int A);
 // Macro arguments included.
 ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2));
@@ -270,12 +270,12 @@
 int D = DEF;
   )cpp";
   ParsedAST AST = TU.build();
-  const std::vector &MacroExpansionLocations = AST.getMacros();
+  auto MacroExpansionLocations = AST.getMacros();
   std::vector MacroExpansionPositions;
   for (const auto &L : MacroExpansionLocations)
-MacroExpansionPositions.push_back(
-sourceLocToPosition(AST.getSourceManager(), L));
-  EXPECT_EQ(MacroExpansionPositions, TestCase.points());
+MacroExpansionPositions.push_back(L.R.start);
+  EXPECT_THAT(MacroExpansionPositions,
+  testing::UnorderedElementsAreArray(TestCase.points()));
 }
 
 } // namespace
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,8 +38,9 @@
 TraverseAST(AST.getASTContext());
 // Add highlightings for macro expansions as they are not traversed by the
 // visitor.
-for (SourceLocation Loc : AST.getMacros())
-  addToken(Loc, HighlightingKind::Macro);
+for (const auto &M : AST.getMacros())
+  Tokens.push_back({HighlightingKind::Macro, M.R});
+// addToken(Loc, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -26,6 +26,7 @@
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Headers.h"
+#include "Macro.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -42,12 +43,6 @@
 /// As we must avoid re-parsing the preamble, any information that can only
 /// be obtained during parsing must be eagerly captured and stored here.
 struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
-   IncludeStructure Includes,
-   std::vector MainFileMacros,
-   std::unique_ptr StatCache,
-   CanonicalIncludes CanonIncludes);
-
   tooling::CompileCommand CompileCommand;
   PrecompiledPreamble Preamble;
   std::vector Diags;
@@ -57,11 +52,17 @@
   // Macros defined in the preamble section of the main file.
   // Users care about headers vs main-file, not preamble vs non-preamble.
   // These should be treated as main-file entities e.g. for

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

2019-09-12 Thread Manikishan Ghantasala via Phabricator via cfe-commits 
Manikishan added a comment.

>>! In D64695#1667696 , 
>>@MyDeveloperDay wrote:

> Does this need landing? given that you have a number of patches in flight 
> perhaps it would be good to request commit access

Yes, it would be good if it is landed. And can I know the procedure for getting 
commit access


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

2019-09-12 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia marked 2 inline comments as done.
Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:5998
+QualType Type = Var->getType();
+if (Type->isSamplerT() || Type->isVoidType())
+  return;

I don't seem to need a check for dependent or auto types because the 
substitution happens using type info rather than getting type from the 
declaration. Not sure if I should explain it here or add the checks just in 
case?



Comment at: clang/lib/Sema/SemaType.cpp:7460
+  // the initializing expression type during the type deduction.
+  (T->isUndeducedAutoType() && IsPointee) ||
   // OpenCL spec v2.0 s6.9.b:

rjmccall wrote:
> Okay, I understand why you're doing this now, and it makes sense.  I would 
> like to propose changing the entire way `deduceOpenCLImplicitAddrSpace` 
> works.  Why don't we do it more like how ObjC ARC infers its implicit 
> ownership qualifiers:
> 
> - In SemaType, we add the default address space to non-qualified, 
> non-dependent, non-undeduced-`auto` pointees when parsing a pointer or 
> reference type.
> - In SemaType, we add the default address space to non-qualified pointees 
> when building a pointer or reference type.
> - We add the default address space at the top level when when building a 
> variable.
> 
> Then all of this context-specific logic where we're looking at different 
> declarator chunks and trying to infer the relationship of the current chunk 
> to the overall type being parsed can just go away or get pushed into a more 
> appropriate position.
Ok, it mainly works, however I still need a bit of parsing horribleness when 
deducing addr space of declarations with parenthesis  in 
`GetFullTypeForDeclarator`. This is the case for block pointers or 
pointers/references to arrays. It is incorrect to add address spaces on 
ParenType while building a pointer or references so I have to detect this as 
special case.


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

https://reviews.llvm.org/D65744



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

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

2019-09-12 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.



> Yes, it would be good if it is landed. And can I know the procedure for 
> getting commit access

https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D67499: [Alignment] Move OffsetToAlignment to Alignment.h

2019-09-12 Thread Guillaume Chatelet via Phabricator via cfe-commits 
gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added subscribers: llvm-commits, cfe-commits, seiya, jsji, atanasyan, 
MaskRay, jrtc27, jakehehrlich, kbarton, hiraditya, nemanjai, sdardis.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: alexshap.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added projects: clang, LLVM.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67499

Files:
  clang/lib/AST/DeclBase.cpp
  llvm/include/llvm/Support/Alignment.h
  llvm/include/llvm/Support/MathExtras.h
  llvm/include/llvm/Support/OnDiskHashTable.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/BranchRelaxation.cpp
  llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/MC/MCAssembler.cpp
  llvm/lib/MC/MachObjectWriter.cpp
  llvm/lib/Object/ArchiveWriter.cpp
  llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
  llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
  llvm/lib/Target/Mips/MipsConstantIslandPass.cpp
  llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
  llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/llvm-cov/TestingSupport.cpp
  llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp

Index: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
===
--- llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
+++ llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "MachOLayoutBuilder.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 
@@ -145,7 +146,7 @@
   Sec.Offset = 0;
 } else {
   uint64_t PaddingSize =
-  OffsetToAlignment(SegFileSize, 1ull << Sec.Align);
+  offsetToAlignment(SegFileSize, llvm::Align(1ull << Sec.Align));
   Sec.Offset = SegOffset + SegFileSize + PaddingSize;
   Sec.Size = Sec.Content.size();
   SegFileSize += PaddingSize + Sec.Size;
Index: llvm/tools/llvm-cov/TestingSupport.cpp
===
--- llvm/tools/llvm-cov/TestingSupport.cpp
+++ llvm/tools/llvm-cov/TestingSupport.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/raw_ostream.h"
@@ -99,7 +100,7 @@
   encodeULEB128(ProfileNamesAddress, OS);
   OS << ProfileNamesData;
   // Coverage mapping data is expected to have an alignment of 8.
-  for (unsigned Pad = OffsetToAlignment(OS.tell(), 8); Pad; --Pad)
+  for (unsigned Pad = offsetToAlignment(OS.tell(), llvm::Align(8)); Pad; --Pad)
 OS.write(uint8_t(0));
   OS << CoverageMappingData;
 
Index: llvm/tools/dsymutil/DwarfStreamer.cpp
===
--- llvm/tools/dsymutil/DwarfStreamer.cpp
+++ llvm/tools/dsymutil/DwarfStreamer.cpp
@@ -339,7 +339,7 @@
 sizeof(int8_t);   // Segment Size (in bytes)
 
 unsigned TupleSize = AddressSize * 2;
-unsigned Padding = OffsetToAlignment(HeaderSize, TupleSize);
+unsigned Padding = offsetToAlignment(HeaderSize, llvm::Align(TupleSize));
 
 Asm->EmitLabelDifference(EndLabel, BeginLabel, 4); // Arange length
 Asm->OutStreamer->EmitLabel(BeginLabel);
Index: llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
===
--- llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
+++ llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
@@ -88,13 +88,13 @@
   const llvm::Align ParentAlign = MBB.getParent()->getAlignment();
 
   if (Align <= ParentAlign)
-return OffsetToAlignment(Offset, Align.value());
+return offsetToAlignment(Offset, Align);
 
   // The alignment of this MBB is larger than the function's alignment, so we
   // can't tell whether or not it will insert nops. Assume that it will.
   if (FirstImpreciseBlock < 0)
 FirstImpreciseBlock = MBB.getNumber();
-  return Align.value() + OffsetToAlignment(Offset, Align.value());
+  return Align.value() + offsetToAlignment(Offset, Align);
 }
 
 /// We need to be careful about the offset of the first block in the function
Index: llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
===
--- llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
+++ llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
@@ -212,11 +212,11 @@
 // element size),

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219917.
kousikk added a comment.

- Add tests and remove the fix inside createFile since it fails in other cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/foodir
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/headerwithdirname.cpp


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: clang-scan-deps dependency
+// CHECK-NEXT: headerwithdirname.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
\ No newline at end of file
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs 
DIR/headerwithdirname.cpp",
+  "file": "DIR/headerwithdirname.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
\ No newline at end of file
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -190,8 +190,11 @@
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
   // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
+  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename)) {
+if (Entry->isDirectory())
+  return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
 return createFile(Entry);
+  }
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: clang-scan-deps dependency
+// CHECK-NEXT: headerwithdirname.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
\ No newline at end of file
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs DIR/headerwithdirname.cpp",
+  "file": "DIR/headerwithdirname.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
\ No newline at end of f

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219918.
kousikk added a comment.

- Add validation inside the createFile() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/foodir
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/headerwithdirname.cpp


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: clang-scan-deps dependency
+// CHECK-NEXT: headerwithdirname.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
\ No newline at end of file
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs 
DIR/headerwithdirname.cpp",
+  "file": "DIR/headerwithdirname.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -190,8 +190,11 @@
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
   // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
+  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename)) {
+if (Entry->isDirectory())
+  return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
 return createFile(Entry);
+  }
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -55,6 +55,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: clang-scan-deps dependency
+// CHECK-NEXT: headerwithdirname.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
\ No newline at end of file
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs DIR/headerwithdirname.cpp",
+  "file": "DIR/headerwithdirname.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

[PATCH] D67091: Fix for headers having the same name as a directory

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

Sorry about the delay on this - I was OOO (back now).

1. I added tests.
2. I couldn't add isDirectory() check to createFile() since that resulted in 
failures to normal scenarios where it was previously passing.

PTAL!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091



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

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

2019-09-12 Thread Yubo Xie via Phabricator via cfe-commits 
xyb created this revision.
xyb added a reviewer: alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Clang-tidy supports output diagnostics from header files if user
specifies --header-filter. But it can't handle relative path well.
For example, the folder structure of a project is:

  // a.h is in /src/a/a.h
  #include "../b/b.h"
  
  // b.h is in /src/b/b.h
  ...
  
  // c.cpp is in /src/c.cpp
  #include "a/a.h"

Now, we set --header-filter as --header-filter=/a/. That means we only
want to check header files under /src/a/ path, and ignore header files
uder /src/b/ path, but in current implementation, clang-tidy will check
/src/b/b.h also, because the name of b.h used in clang-tidy is
/src/a/../b/b.h.

This change tries to fix this issue.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D67501

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/test/clang-tidy/Inputs/file-filter/subfolder_a/header.h
  clang-tools-extra/test/clang-tidy/Inputs/file-filter/subfolder_b/header.h
  clang-tools-extra/test/clang-tidy/Inputs/file-filter/subfolder_c/header.h
  clang-tools-extra/test/clang-tidy/file-filter.cpp

Index: clang-tools-extra/test/clang-tidy/file-filter.cpp
===
--- clang-tools-extra/test/clang-tidy/file-filter.cpp
+++ clang-tools-extra/test/clang-tidy/file-filter.cpp
@@ -9,6 +9,12 @@
 //   file-filter\header*.h due to code order between '/' and '\\'.
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4 %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers -quiet %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4-QUIET %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_a' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_a' -quiet %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5-QUIET %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_b' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK6 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_b' -quiet %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK6-QUIET %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_c' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK7 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='subfolder_c' -quiet %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK7-QUIET %s
 
 #include "header1.h"
 // CHECK-NOT: warning:
@@ -19,6 +25,12 @@
 // CHECK3-QUIET-NOT: warning:
 // CHECK4: header1.h:1:12: warning: single-argument constructors
 // CHECK4-QUIET: header1.h:1:12: warning: single-argument constructors
+// CHECK5-NOT: warning:
+// CHECK5-QUIET-NOT: warning:
+// CHECK6-NOT: warning:
+// CHECK6-QUIET-NOT: warning:
+// CHECK7-NOT: warning:
+// CHECK7-QUIET-NOT: warning:
 
 #include "header2.h"
 // CHECK-NOT: warning:
@@ -29,6 +41,44 @@
 // CHECK3-QUIET: header2.h:1:12: warning: single-argument constructors
 // CHECK4: header2.h:1:12: warning: single-argument constructors
 // CHECK4-QUIET: header2.h:1:12: warning: single-argument constructors
+// CHECK5-NOT: warning:
+// CHECK5-QUIET-NOT: warning:
+// CHECK6-NOT: warning:
+// CHECK6-QUIET-NOT: warning:
+// CHECK7-NOT: warning:
+// CHECK7-QUIET-NOT: warning:
+
+#include "subfolder_a/header.h"
+// CHECK-NOT: warning:
+// CHECK-QUIET-NOT: warning:
+// CHECK2: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK2-QUIET: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK3-NOT: warning:
+// CHECK3-QUIET-NOT: warning:
+// CHECK4: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK4-QUIET: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK5: header.h:3:12: warning: single-argument constructors must be marked explicit
+// CHECK5-QUIET: header.h:3:12: warning: single-argument constructors must be marked explicit
+// CHECK6: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK6-QUIET: header.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK7-NOT: warning:
+// CHECK7-QUIE

[PATCH] D67499: [Alignment] Move OffsetToAlignment to Alignment.h

2019-09-12 Thread Clement Courbet via Phabricator via cfe-commits 
courbet added inline comments.



Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1024
 NextBlockOffset = BBInfo[Water->getNumber()].postOffset();
-NextBlockLogAlignment = 0;
+NextBlockAlignment = llvm::Align();
   } else {

this statement is now useless.



Comment at: llvm/lib/Target/Mips/MipsSERegisterInfo.cpp:216
+const llvm::Align OffsetAlign =
+llvm::Align(getLoadStoreOffsetAlign(MI.getOpcode()));
 

what about:

```
const llvm::Align OffsetAlign =
llvm::Align(llvm::Align(getLoadStoreOffsetAlign(MI.getOpcode();
```

:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67499



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

[PATCH] D67499: [Alignment] Move OffsetToAlignment to Alignment.h

2019-09-12 Thread Guillaume Chatelet via Phabricator via cfe-commits 
gchatelet updated this revision to Diff 219923.
gchatelet marked an inline comment as done.
gchatelet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67499

Files:
  clang/lib/AST/DeclBase.cpp
  llvm/include/llvm/Support/Alignment.h
  llvm/include/llvm/Support/MathExtras.h
  llvm/include/llvm/Support/OnDiskHashTable.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/BranchRelaxation.cpp
  llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/MC/MCAssembler.cpp
  llvm/lib/MC/MachObjectWriter.cpp
  llvm/lib/Object/ArchiveWriter.cpp
  llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
  llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
  llvm/lib/Target/Mips/MipsConstantIslandPass.cpp
  llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
  llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/llvm-cov/TestingSupport.cpp
  llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp

Index: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
===
--- llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
+++ llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "MachOLayoutBuilder.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 
@@ -145,7 +146,7 @@
   Sec.Offset = 0;
 } else {
   uint64_t PaddingSize =
-  OffsetToAlignment(SegFileSize, 1ull << Sec.Align);
+  offsetToAlignment(SegFileSize, llvm::Align(1ull << Sec.Align));
   Sec.Offset = SegOffset + SegFileSize + PaddingSize;
   Sec.Size = Sec.Content.size();
   SegFileSize += PaddingSize + Sec.Size;
Index: llvm/tools/llvm-cov/TestingSupport.cpp
===
--- llvm/tools/llvm-cov/TestingSupport.cpp
+++ llvm/tools/llvm-cov/TestingSupport.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/raw_ostream.h"
@@ -99,7 +100,7 @@
   encodeULEB128(ProfileNamesAddress, OS);
   OS << ProfileNamesData;
   // Coverage mapping data is expected to have an alignment of 8.
-  for (unsigned Pad = OffsetToAlignment(OS.tell(), 8); Pad; --Pad)
+  for (unsigned Pad = offsetToAlignment(OS.tell(), llvm::Align(8)); Pad; --Pad)
 OS.write(uint8_t(0));
   OS << CoverageMappingData;
 
Index: llvm/tools/dsymutil/DwarfStreamer.cpp
===
--- llvm/tools/dsymutil/DwarfStreamer.cpp
+++ llvm/tools/dsymutil/DwarfStreamer.cpp
@@ -339,7 +339,7 @@
 sizeof(int8_t);   // Segment Size (in bytes)
 
 unsigned TupleSize = AddressSize * 2;
-unsigned Padding = OffsetToAlignment(HeaderSize, TupleSize);
+unsigned Padding = offsetToAlignment(HeaderSize, llvm::Align(TupleSize));
 
 Asm->EmitLabelDifference(EndLabel, BeginLabel, 4); // Arange length
 Asm->OutStreamer->EmitLabel(BeginLabel);
Index: llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
===
--- llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
+++ llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
@@ -88,13 +88,13 @@
   const llvm::Align ParentAlign = MBB.getParent()->getAlignment();
 
   if (Align <= ParentAlign)
-return OffsetToAlignment(Offset, Align.value());
+return offsetToAlignment(Offset, Align);
 
   // The alignment of this MBB is larger than the function's alignment, so we
   // can't tell whether or not it will insert nops. Assume that it will.
   if (FirstImpreciseBlock < 0)
 FirstImpreciseBlock = MBB.getNumber();
-  return Align.value() + OffsetToAlignment(Offset, Align.value());
+  return Align.value() + offsetToAlignment(Offset, Align);
 }
 
 /// We need to be careful about the offset of the first block in the function
Index: llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
===
--- llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
+++ llvm/lib/Target/Mips/MipsSERegisterInfo.cpp
@@ -212,11 +212,9 @@
 // element size), otherwise it is a 16-bit signed immediate.
 unsigned OffsetBitSize =
 getLoadStoreOffsetSizeInBits(MI.getOpcode(), MI.getOperand(OpNo - 1));
-unsigned OffsetAlign = getLoadStoreOffsetAlign(MI.getOpcode());
-
+const llvm::Align OffsetAlign(getLoadStoreOffsetAlign(MI.getOpcode()));
 if (OffsetBitSize < 16 && isInt<16>(Offset) &&
-(!isIntN(OffsetBitSize, Offset) ||
- OffsetToAlignment(Offset, Offs

[PATCH] D67499: [Alignment] Move OffsetToAlignment to Alignment.h

2019-09-12 Thread Guillaume Chatelet via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371742: [Alignment] Move OffsetToAlignment to Alignment.h 
(authored by gchatelet, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D67499?vs=219923&id=219925#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67499

Files:
  cfe/trunk/lib/AST/DeclBase.cpp
  llvm/trunk/include/llvm/Support/Alignment.h
  llvm/trunk/include/llvm/Support/MathExtras.h
  llvm/trunk/include/llvm/Support/OnDiskHashTable.h
  llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/trunk/lib/CodeGen/BranchRelaxation.cpp
  llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
  llvm/trunk/lib/MC/ELFObjectWriter.cpp
  llvm/trunk/lib/MC/MCAssembler.cpp
  llvm/trunk/lib/MC/MachObjectWriter.cpp
  llvm/trunk/lib/Object/ArchiveWriter.cpp
  llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
  llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
  llvm/trunk/lib/Target/Mips/MipsConstantIslandPass.cpp
  llvm/trunk/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/trunk/lib/Target/Mips/MipsSERegisterInfo.cpp
  llvm/trunk/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/trunk/tools/dsymutil/DwarfStreamer.cpp
  llvm/trunk/tools/llvm-cov/TestingSupport.cpp
  llvm/trunk/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp

Index: llvm/trunk/include/llvm/Support/MathExtras.h
===
--- llvm/trunk/include/llvm/Support/MathExtras.h
+++ llvm/trunk/include/llvm/Support/MathExtras.h
@@ -712,13 +712,6 @@
   return (Value - Skew) / Align * Align + Skew;
 }
 
-/// Returns the offset to the next integer (mod 2**64) that is greater than
-/// or equal to \p Value and is a multiple of \p Align. \p Align must be
-/// non-zero.
-inline uint64_t OffsetToAlignment(uint64_t Value, uint64_t Align) {
-  return alignTo(Value, Align) - Value;
-}
-
 /// Sign-extend the number in the bottom B bits of X to a 32-bit integer.
 /// Requires 0 < B <= 32.
 template  constexpr inline int32_t SignExtend32(uint32_t X) {
Index: llvm/trunk/include/llvm/Support/Alignment.h
===
--- llvm/trunk/include/llvm/Support/Alignment.h
+++ llvm/trunk/include/llvm/Support/Alignment.h
@@ -133,6 +133,12 @@
   return A ? alignTo(Size, A.getValue()) : Size;
 }
 
+/// Returns the offset to the next integer (mod 2**64) that is greater than
+/// or equal to \p Value and is a multiple of \p Align.
+inline uint64_t offsetToAlignment(uint64_t Value, llvm::Align Align) {
+  return alignTo(Value, Align) - Value;
+}
+
 /// Returns the log2 of the alignment.
 inline unsigned Log2(Align A) { return A.ShiftValue; }
 
Index: llvm/trunk/include/llvm/Support/OnDiskHashTable.h
===
--- llvm/trunk/include/llvm/Support/OnDiskHashTable.h
+++ llvm/trunk/include/llvm/Support/OnDiskHashTable.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_SUPPORT_ONDISKHASHTABLE_H
 #define LLVM_SUPPORT_ONDISKHASHTABLE_H
 
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/EndianStream.h"
@@ -207,7 +208,8 @@
 
 // Pad with zeros so that we can start the hashtable at an aligned address.
 offset_type TableOff = Out.tell();
-uint64_t N = llvm::OffsetToAlignment(TableOff, alignof(offset_type));
+uint64_t N =
+llvm::offsetToAlignment(TableOff, llvm::Align(alignof(offset_type)));
 TableOff += N;
 while (N--)
   LE.write(0);
Index: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
===
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -17,6 +17,7 @@
 #include "RuntimeDyldMachO.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Support/Alignment.h"
 #include "llvm/Support/MSVCErrorWorkarounds.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
@@ -737,7 +738,8 @@
   return NameOrErr.takeError();
 if (Align) {
   // This symbol has an alignment requirement.
-  uint64_t AlignOffset = OffsetToAlignment((uint64_t)Addr, Align);
+  uint64_t AlignOffset =
+  offsetToAlignment((uint64_t)Addr, llvm::Align(Align));
   Addr += AlignOffset;
   Offset += AlignOffset;
 }
Index: llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
===
--- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
+++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -1016,14 +1016,14 @@
   BBInfoVector &BBInfo = BBUtils->getBBInfo();
   unsigned CPELogAlign = getCPELogAlign(U.CPEMI);
   unsigned CPEOffset = BBInfo[Water->getNumber()].postOffset(CPELogAlign);
-

r371742 - [Alignment] Move OffsetToAlignment to Alignment.h

2019-09-12 Thread Guillaume Chatelet via cfe-commits 
Author: gchatelet
Date: Thu Sep 12 08:20:36 2019
New Revision: 371742

URL: http://llvm.org/viewvc/llvm-project?rev=371742&view=rev
Log:
[Alignment] Move OffsetToAlignment to Alignment.h

Summary:
This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

Reviewers: courbet, JDevlieghere, alexshap, rupprecht, jhenderson

Subscribers: sdardis, nemanjai, hiraditya, kbarton, jakehehrlich, jrtc27, 
MaskRay, atanasyan, jsji, seiya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

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

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=371742&r1=371741&r2=371742&view=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Thu Sep 12 08:20:36 2019
@@ -100,7 +100,7 @@ void *Decl::operator new(std::size_t Siz
 // Ensure required alignment of the resulting object by adding extra
 // padding at the start if required.
 size_t ExtraAlign =
-llvm::OffsetToAlignment(sizeof(Module *), alignof(Decl));
+llvm::offsetToAlignment(sizeof(Module *), llvm::Align(alignof(Decl)));
 auto *Buffer = reinterpret_cast(
 ::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, Ctx));
 Buffer += ExtraAlign;


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

[PATCH] D52524: Add -Wpoison-system-directories warning

2019-09-12 Thread Denis Nikitin via Phabricator via cfe-commits 
denik updated this revision to Diff 219924.
denik added a comment.

Combined two if into one.


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

https://reviews.llvm.org/D52524

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/lib/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/include/c++/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/lib/gcc/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/include/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/lib/.keep
  clang/test/Frontend/warning-poison-system-directories.c


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warning-poison-system-directories.c
@@ -0,0 +1,27 @@
+// System directory and sysroot option causes warning.
+// RUN: %clang -Wpoison-system-directories -target x86_64 -I/usr/include 
--sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-cxx-isystem/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c 
-o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-iquote/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-isystem/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s
+
+// With -Werror the warning causes the failure.
+// RUN: not %clang -Werror=poison-system-directories -target x86_64 
-I/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 
2> %t.3.stderr
+// RUN: FileCheck -check-prefix=ERROR < %t.3.stderr %s
+
+// Cros target without sysroot causes no warning.
+// RUN: %clang -Wpoison-system-directories -Werror -target x86_64 
-I/usr/include -c -o - %s
+
+// By default the warning is off.
+// RUN: %clang -Werror -target x86_64 -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s
+
+// WARN: warning: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Wpoison-system-directories]
+
+// ERROR: error: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Werror,-Wpoison-system-directories]
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -137,6 +137,13 @@
   SmallString<256> MappedPathStorage;
   StringRef MappedPathStr = Path.toStringRef(MappedPathStorage);
 
+  // If use system headers while cross-compiling, emit the warning.
+  if (HasSysroot && (MappedPathStr.startswith("/usr/include") ||
+ MappedPathStr.startswith("/usr/local/include"))) {
+Headers.getDiags().Report(diag::warn_poison_system_directories)
+<< MappedPathStr;
+  }
+
   // Compute the DirectoryLookup type.
   SrcMgr::CharacteristicKind Type;
   if (Group == Quoted || Group == Angled || Group == IndexHeaderMap) {
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -315,4 +315,9 @@
 "no analyzer checkers or packages are associated with '%0'">;
 def note_suggest_disabling_all_checkers : Note<
 "use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;
+
+// Poison system directories.
+def warn_poison_system_directories : Warning <
+  "include location '%0' is unsafe for cross-compilation">,
+  InGroup>, DefaultIgnore;
 }


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warning-poison-system-directories.c
@@ -0,0 +1,27 @@
+// System directory and sysroot option causes warning.
+// RUN: %clang -Wpoison-system-directories -target x86_64 -I/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 -cxx-isystem/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_li

[PATCH] D52524: Add -Wpoison-system-directories warning

2019-09-12 Thread Denis Nikitin via Phabricator via cfe-commits 
denik marked 2 inline comments as done.
denik added inline comments.



Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:141-143
+  if (HasSysroot) {
+if (MappedPathStr.startswith("/usr/include") ||
+MappedPathStr.startswith("/usr/local/include")) {

aaron.ballman wrote:
> These should be combined into a single if statement:
> ```
> if (HasSysroot && (MappedPathStr.startswith(...) || 
> MappedPathStr.startswith(...))) {
> ```
Done.


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

https://reviews.llvm.org/D52524



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Sergey Dmitriev via Phabricator via cfe-commits 
sdmitriev added a comment.

In D64943#1667469 , @JonChesterfield 
wrote:

> I'm on board with getting rid of the linker script. Gold's limited support 
> for that seems conclusive.
>
> I believe the current script does two things:
>  1/ takes a binary and embeds it in a section named 
> .omp_offloading.amdgcn-amd-amdhsa
>  2/ provides start/end symbols for that section and for 
> .omp_offloading.entries.
>
> 2/ is discussed above.
>  1/ can be implemented as a call to (llvm-)objcopy
>
> > If binary is used as the value for --input-target, the input file will be 
> > embedded as a data section in an ELF relocatable object, with symbols 
> > _binary__start, _binary__end, and 
> > _binary__size representing the start, end and size of the data, 
> > where  is the path of the input file as specified on the command 
> > line with non-alphanumeric characters converted to _.
>
> I think dropping the linker script means that cmake will need to invoke an 
> extra executable. As far as I can see, that tool can be objcopy instead of 
> clang-offload-wrapper.
>
> Does this diff mix getting rid of the linker script with other changes? E.g. 
> it looks like the metadata generation is moving from clang to the new tool, 
> but that seems orthogonal to dropping the linker script.


Metadata is still generated by the clang, there are no changes in this area. 
What is moving to a wrapper tool is the generation of the offload registration 
code. Let me just attach the slides that I presented on the inter company 
meeting were the proposal was discussed. It'll probably answer most of your 
questions. F9983224: openmp_linker_script.pdf 



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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

>> Does this diff mix getting rid of the linker script with other changes? E.g. 
>> it looks like the metadata generation is moving from clang to the new tool, 
>> but that seems orthogonal to dropping the linker script.
> 
> Metadata is still generated by the clang, there are no changes in this area. 
> What is moving to a wrapper tool is the generation of the offload 
> registration code. Let me just attach the slides that I presented on the 
> inter company meeting were the proposal was discussed. It'll probably answer 
> most of your questions. F9983224: openmp_linker_script.pdf 
> 

It does indeed, thanks. I see the motivation for delaying offload registration 
code. I'm pretty sure that is indeed orthogonal to removing the linker script.

How would you feel about using objcopy to embed the device binary?


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

https://reviews.llvm.org/D64943



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

2019-09-12 Thread Kuan Hsu Chen via Phabricator via cfe-commits 
khchen updated this revision to Diff 219941.
khchen added a comment.

@lenary Yes, you are right. add LTO support for Linux platform.


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

https://reviews.llvm.org/D67409

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/Driver/gold-lto.c

Index: clang/test/Driver/gold-lto.c
===
--- clang/test/Driver/gold-lto.c
+++ clang/test/Driver/gold-lto.c
@@ -26,3 +26,21 @@
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
 // CHECK-X86-ANDROID: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+//
+// RUN: %clang -target riscv64-unknown-elf -### %t.o -flto 2>&1 \
+// RUN: -march=rv64imf -mabi=lp64f \
+// RUN: | FileCheck %s --check-prefix=CHECK-RISCV-BAREMETAL
+// CHECK-RISCV-BAREMETAL: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=--mattr=+m"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=--mattr=+f"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=--mattr=+relax"
+// CHECK-RISCV-BAREMETAL: "-plugin-opt=-target-abi=lp64f"
+//
+// RUN: %clang -target riscv64-unknown-linux-gnu -### %t.o -flto 2>&1 \
+// RUN: -march=rv64imf -mabi=lp64f \
+// RUN: | FileCheck %s --check-prefix=CHECK-RISCV-LINUX
+// CHECK-RISCV-LINUX: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
+// CHECK-RISCV-LINUX: "-plugin-opt=--mattr=+m"
+// CHECK-RISCV-LINUX: "-plugin-opt=--mattr=+f"
+// CHECK-RISCV-LINUX: "-plugin-opt=--mattr=+relax"
+// CHECK-RISCV-LINUX: "-plugin-opt=-target-abi=lp64f"
Index: clang/lib/Driver/ToolChains/RISCVToolchain.h
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.h
+++ clang/lib/Driver/ToolChains/RISCVToolchain.h
@@ -25,6 +25,7 @@
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
  Action::OffloadKind) const override;
+  bool HasNativeLLVMSupport() const override { return true; }
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "RISCVToolchain.h"
+#include "Arch/RISCV.h"
 #include "CommonArgs.h"
 #include "InputInfo.h"
 #include "clang/Driver/Compilation.h"
@@ -100,6 +101,13 @@
 
   std::string Linker = getToolChain().GetProgramPath(getShortName());
 
+  if (D.isUsingLTO()) {
+assert(!Inputs.empty() && "Must have at least one input.");
+AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
+  D.getLTOMode() == LTOK_Thin);
+tools::riscv::addGoldOptions(ToolChain, Args, CmdArgs);
+  }
+
   bool WantCRTs =
   !Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles);
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -514,6 +514,15 @@
 assert(!Inputs.empty() && "Must have at least one input.");
 AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
   D.getLTOMode() == LTOK_Thin);
+switch (ToolChain.getArch()) {
+default:
+  break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64: {
+  tools::riscv::addGoldOptions(ToolChain, Args, CmdArgs);
+  break;
+}
+}
   }
 
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
Index: clang/lib/Driver/ToolChains/Arch/RISCV.h
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.h
+++ clang/lib/Driver/ToolChains/Arch/RISCV.h
@@ -23,6 +23,8 @@
 std::vector &Features);
 StringRef getRISCVABI(const llvm::opt::ArgList &Args,
   const llvm::Triple &Triple);
+void addGoldOptions(const ToolChain &ToolChain, const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs);
 } // end namespace riscv
 } // namespace tools
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -383,3 +383,42 @@
   // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
   return Triple.

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

2019-09-12 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:387
+
+void riscv::addGoldOptions(const ToolChain &ToolChain,
+   const llvm::opt::ArgList &Args,

gold doesn't support RISC-V, does it?


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

https://reviews.llvm.org/D67409



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

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

2019-09-12 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane marked an inline comment as done.
erichkeane added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2266
   let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">,
-   C2x<"", "maybe_unused">];
+   C2x<"", "maybe_unused">, Pragma<"", "unused">];
   let Subjects = SubjectList<[Var, ObjCIvar, Type, Enum, EnumConstant, Label,

aaron.ballman wrote:
> I'm surprised to see this as part of an NFC refactoring. Hopefully it will 
> become more obvious later. :-D
Yep, was a separate bug I found while going through things.  I'll take it out 
and do it later.



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:66-69
+  unsigned AttrKind : 16;
+  /// Corresponds to the Syntax enum.
+  unsigned SyntaxUsed : 3;
+  unsigned SpellingIndex : 4;

aaron.ballman wrote:
> Do you want to do `= 0` here to give them in-class initializers? (well, 
> `SpellingIndex` should probably be `SpellingNotCalculated`).
Bitfield in-class initializers are a C++2a feature.


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

https://reviews.llvm.org/D67368



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

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

In D67420#1666578 , @NoQ wrote:

> In D67420#1666141 , @Szelethus wrote:
>
> > I do!
>
>
> Hmm, it sounds like you want to force both Clang frontend and Clang-Tidy to 
> use the same set of flags to control these options (?) Much like 
> `-analyzer-config`, these flags will have different "style" compared to other 
> flags in the tool. Which is probably not too bad for hidden frontend flags 
> that control the Analyzer because they're anyway set by a separate GUI 
> checkbox, but the inconsistency with other flags would be super glaring in 
> case of Clang-Tidy CLI. Do we really want to go in that direction? I'll be 
> much more comfortable with letting each tool deal with its flags the way it 
> prefers - this doesn't look like a good place for code reuse to me.


There are two things I wanted to touch on, but kinda failed to emphasize it. 
First is the topic of whether `-analyzer-config` flags are fitting for a 
feature no longer specific to the analyzer, and the second is similar in 
nature, but in the actual code -- it doesn't doesn't feel natural to me that 
`AnalyzerOptions` is required to construct this, while at the same time we're 
trying to make diagnostics construction independent of the analyzer. But I 
really wouldn't like to overengineer this just for the sake of it :^)

On the first, I'm kinda meh, even if we went for it, it would be a separate 
revision I guess, but the second has me concerned, unless I'm not following 
something right.


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

https://reviews.llvm.org/D67420



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

2019-09-12 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon added a comment.

In D64146#1667567 , @nand wrote:

> Thanks for looking into the problem - sorry for the delay!


No problem, you still need a specialist to approve the patch though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

r371749 - Don't warn about selectany on implicitly inline variables

2019-09-12 Thread Reid Kleckner via cfe-commits 
Author: rnk
Date: Thu Sep 12 10:55:48 2019
New Revision: 371749

URL: http://llvm.org/viewvc/llvm-project?rev=371749&view=rev
Log:
Don't warn about selectany on implicitly inline variables

Summary:
This avoids a -Wignored-attribute warning on the code pattern Microsoft
recommends for integral const static data members defined in headers
here:
https://docs.microsoft.com/en-us/cpp/build/reference/microsoft-extensions-to-c-and-cpp?view=vs-2019

The attribute is redundant, but it is necessary when compiling in C++14
modes with /Za, which disables MSVC's extension that treats such
variables as implicitly inline.

Fixes PR43270

Reviewers: epastor, thakis, hans

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/SemaCXX/declspec-selectany.cpp
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=371749&r1=371748&r2=371749&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Sep 12 10:55:48 2019
@@ -2652,6 +2652,15 @@ static void checkNewAttributesAfterDef(S
 --E;
 continue;
   }
+} else if (isa(NewAttribute) &&
+   cast(New)->isInline() &&
+   !cast(New)->isInlineSpecified()) {
+  // Don't warn about applying selectany to implicitly inline variables.
+  // Older compilers and language modes would require the use of selectany
+  // to make such variables inline, and it would have no effect if we
+  // honored it.
+  ++I;
+  continue;
 }
 
 S.Diag(NewAttribute->getLocation(),

Added: cfe/trunk/test/SemaCXX/declspec-selectany.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/declspec-selectany.cpp?rev=371749&view=auto
==
--- cfe/trunk/test/SemaCXX/declspec-selectany.cpp (added)
+++ cfe/trunk/test/SemaCXX/declspec-selectany.cpp Thu Sep 12 10:55:48 2019
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-scei-ps4 -fdeclspec -verify
+
+// MSVC emits this error too.
+const int __declspec(selectany) test1 = 0; // expected-error {{'selectany' can 
only be applied to data items with external linkage}}
+
+extern const int test2;
+const int test2 = 42; // expected-note {{previous definition is here}}
+extern __declspec(selectany) const int test2; // expected-warning {{attribute 
declaration must precede definition}}
+
+extern const int test3;
+const int __declspec(selectany) test3 = 42; // Standard usage.
+
+struct Test4 {
+  static constexpr int sdm = 0;
+};
+__declspec(selectany) constexpr int Test4::sdm; // no warning


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

[PATCH] D67426: Don't warn about selectany on implicitly inline variables

2019-09-12 Thread Reid Kleckner via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb00a49d1b3a1: Don't warn about selectany on implicitly 
inline variables (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67426

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/declspec-selectany.cpp


Index: clang/test/SemaCXX/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/declspec-selectany.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-scei-ps4 -fdeclspec -verify
+
+// MSVC emits this error too.
+const int __declspec(selectany) test1 = 0; // expected-error {{'selectany' can 
only be applied to data items with external linkage}}
+
+extern const int test2;
+const int test2 = 42; // expected-note {{previous definition is here}}
+extern __declspec(selectany) const int test2; // expected-warning {{attribute 
declaration must precede definition}}
+
+extern const int test3;
+const int __declspec(selectany) test3 = 42; // Standard usage.
+
+struct Test4 {
+  static constexpr int sdm = 0;
+};
+__declspec(selectany) constexpr int Test4::sdm; // no warning
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2652,6 +2652,15 @@
 --E;
 continue;
   }
+} else if (isa(NewAttribute) &&
+   cast(New)->isInline() &&
+   !cast(New)->isInlineSpecified()) {
+  // Don't warn about applying selectany to implicitly inline variables.
+  // Older compilers and language modes would require the use of selectany
+  // to make such variables inline, and it would have no effect if we
+  // honored it.
+  ++I;
+  continue;
 }
 
 S.Diag(NewAttribute->getLocation(),


Index: clang/test/SemaCXX/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/declspec-selectany.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++17 %s -triple x86_64-windows-msvc -fdeclspec -verify
+// RUN: %clang_cc1 -std=c++14 %s -triple x86_64-scei-ps4 -fdeclspec -verify
+
+// MSVC emits this error too.
+const int __declspec(selectany) test1 = 0; // expected-error {{'selectany' can only be applied to data items with external linkage}}
+
+extern const int test2;
+const int test2 = 42; // expected-note {{previous definition is here}}
+extern __declspec(selectany) const int test2; // expected-warning {{attribute declaration must precede definition}}
+
+extern const int test3;
+const int __declspec(selectany) test3 = 42; // Standard usage.
+
+struct Test4 {
+  static constexpr int sdm = 0;
+};
+__declspec(selectany) constexpr int Test4::sdm; // no warning
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2652,6 +2652,15 @@
 --E;
 continue;
   }
+} else if (isa(NewAttribute) &&
+   cast(New)->isInline() &&
+   !cast(New)->isInlineSpecified()) {
+  // Don't warn about applying selectany to implicitly inline variables.
+  // Older compilers and language modes would require the use of selectany
+  // to make such variables inline, and it would have no effect if we
+  // honored it.
+  ++I;
+  continue;
 }
 
 S.Diag(NewAttribute->getLocation(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Sergey Dmitriev via Phabricator via cfe-commits 
sdmitriev added a comment.

In D64943#1668006 , @JonChesterfield 
wrote:

> >> Does this diff mix getting rid of the linker script with other changes? 
> >> E.g. it looks like the metadata generation is moving from clang to the new 
> >> tool, but that seems orthogonal to dropping the linker script.
> > 
> > Metadata is still generated by the clang, there are no changes in this 
> > area. What is moving to a wrapper tool is the generation of the offload 
> > registration code. Let me just attach the slides that I presented on the 
> > inter company meeting were the proposal was discussed. It'll probably 
> > answer most of your questions. F9983224: openmp_linker_script.pdf 
> > 
>
> It does indeed, thanks. I see the motivation for delaying offload 
> registration code. I'm pretty sure that is indeed orthogonal to removing the 
> linker script.
>
> How would you feel about using objcopy to embed the device binary?


I see some problems with using llvm-objcopy for that. First issue is that 
symbols created by llvm-objcopy for embedded data depend on the input file 
name. As you know these symbols are referenced from the offload registration 
code that is currently added to an object by the clang at compile time. I not 
sure how you can guarantee that symbol names will match. And another, more 
important problem is that it won't work on Windows because llvm-objcopy 
produces ELF object according to the description.

Anyway I am going to change entries section name to "omp_offloading_entries", 
remove omptargetbegin.o/omptargetend.o and upload the revised patch.


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

https://reviews.llvm.org/D64943



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

r371751 - [clang-scan-deps] remove dots and dots dots from the reported file dependency paths

2019-09-12 Thread Alex Lorenz via cfe-commits 
Author: arphaman
Date: Thu Sep 12 11:03:24 2019
New Revision: 371751

URL: http://llvm.org/viewvc/llvm-project?rev=371751&view=rev
Log:
[clang-scan-deps] remove dots and dots dots from the reported file dependency 
paths

This resolves differences observed on LLVM + Clang when running the comparison 
between canonical
dependencies (full preprocessing, no file manager reused), and dependencies 
obtained
when the file manager was reused between the full preprocessing invocations.

Modified:
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json

Modified: cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp?rev=371751&r1=371750&r2=371751&view=diff
==
--- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
(original)
+++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp Thu 
Sep 12 11:03:24 2019
@@ -30,8 +30,12 @@ public:
   : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
 
   void finishedMainFile(DiagnosticsEngine &Diags) override {
-for (const auto &File : getDependencies())
-  C.handleFileDependency(*Opts, File);
+llvm::SmallString<256> CanonPath;
+for (const auto &File : getDependencies()) {
+  CanonPath = File;
+  llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
+  C.handleFileDependency(*Opts, CanonPath);
+}
   }
 
 private:

Modified: 
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json?rev=371751&r1=371750&r2=371751&view=diff
==
--- 
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json 
(original)
+++ 
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json 
Thu Sep 12 11:03:24 2019
@@ -6,7 +6,7 @@
 },
 {
   "directory": "DIR",
-  "command": "clang -E DIR/subframework_header_dir_symlink_input2.m 
-FInputs/frameworks -iframework Inputs/frameworks_symlink",
+  "command": "clang -E DIR/subframework_header_dir_symlink_input2.m 
-FInputs/frameworks -iframework 
Inputs/frameworks_symlink/../frameworks_symlink",
   "file": "DIR/subframework_header_dir_symlink_input2.m"
 }
 ]


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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-12 Thread Jon Chesterfield via Phabricator via cfe-commits 
JonChesterfield added a comment.

> I see some problems with using llvm-objcopy for that. First issue is that 
> symbols created by llvm-objcopy for embedded data depend on the input file 
> name. As you know these symbols are referenced from the offload registration 
> code that is currently added to an object by the clang at compile time. I not 
> sure how you can guarantee that symbol names will match.

That seems solvable by renaming the input file / passing a string to clang.

> And another, more important problem is that it won't work on Windows because 
> llvm-objcopy produces ELF object according to the description.

objcopy works with coff in the meantime, and we already need a bunch of unix 
tools to build llvm on windows.

> Anyway I am going to change entries section name to "omp_offloading_entries", 
> remove omptargetbegin.o/omptargetend.o and upload the revised patch.

Thanks!


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

https://reviews.llvm.org/D64943



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

Re: r371663 - Start porting ivfsoverlay tests to Windows

2019-09-12 Thread Reid Kleckner via cfe-commits 
Thanks, that seems like the correct fix. The property that makes them fail
is the native system path style, not the default target.

On Thu, Sep 12, 2019 at 5:03 AM Jeremy Morse 
wrote:

> On Thu, Sep 12, 2019 at 11:24 AM Jeremy Morse
>  wrote:
> > Assuming no-one else is looking at this, I'll commit a fix in ~30 mins.
>
> r371726
>
> --
> Thanks,
> Jeremy
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371753 - [MS] Warn when shadowing template parameters under -fms-compatibility

2019-09-12 Thread Reid Kleckner via cfe-commits 
Author: rnk
Date: Thu Sep 12 11:26:34 2019
New Revision: 371753

URL: http://llvm.org/viewvc/llvm-project?rev=371753&view=rev
Log:
[MS] Warn when shadowing template parameters under -fms-compatibility

Summary:
C++ does not allow shadowing template parameters, but previously we
allowed it under -fms-extensions. Now this behavior is controlled by
-fms-compatibility, and we emit a -Wmicrosoft-template warning when it
happens.

Fixes PR43265

Reviewers: thakis, hans

Subscribers: amccarth, rsmith, STL_MSFT, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371753&r1=371752&r2=371753&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 12 11:26:34 
2019
@@ -4000,6 +4000,8 @@ def err_ovl_no_viable_literal_operator :
 // C++ Template Declarations
 def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
+def ext_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Text>, InGroup;
 def note_template_param_here : Note<"template parameter is declared here">;
 def warn_template_export_unsupported : Warning<
   "exported templates are unsupported">;

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=371753&r1=371752&r2=371753&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Thu Sep 12 11:26:34 2019
@@ -849,15 +849,14 @@ bool Sema::DiagnoseUninstantiableTemplat
 void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
-  // Microsoft Visual C++ permits template parameters to be shadowed.
-  if (getLangOpts().MicrosoftExt)
-return;
-
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
-  Diag(Loc, diag::err_template_param_shadow)
-<< cast(PrevDecl)->getDeclName();
+  //
+  // Make this a warning when MSVC compatibility is requested.
+  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
+ : diag::err_template_param_shadow;
+  Diag(Loc, DiagId) << cast(PrevDecl)->getDeclName();
   Diag(PrevDecl->getLocation(), diag::note_template_param_here);
 }
 

Modified: cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/DelayedTemplateParsing.cpp?rev=371753&r1=371752&r2=371753&view=diff
==
--- cfe/trunk/test/Parser/DelayedTemplateParsing.cpp (original)
+++ cfe/trunk/test/Parser/DelayedTemplateParsing.cpp Thu Sep 12 11:26:34 2019
@@ -48,22 +48,6 @@ template  void foo5() {} // exp
 
   
 
-namespace Inner_Outer_same_template_param_name {  
-
-template 
-class Outmost {
-public:
-template 
-class Inner {
-public:
-void f() {
-T* var;
-}
-   };
-};
-
-}
-
 
 namespace PR11931 {
 

Modified: cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp?rev=371753&r1=371752&r2=371753&view=diff
==
--- cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp Thu Sep 12 11:26:34 2019
@@ -366,3 +366,21 @@ struct S {
 int S::fn() { return 0; } // expected-warning {{is missing exception 
specification}}
 }
 
+namespace PR43265 {
+template  // expected-note {{template parameter is declared here}}
+struct Foo {
+  static const int N = 42; // expected-warning {{declaration of 'N' shadows 
template parameter}}
+};
+}
+
+namespace Inner_Outer_same_template_param_name {
+template  // expected-note {{template parameter is declared here}}
+struct Outmost {
+  template  // expected-warning {{declaration of 'T' shadows 
template parameter}}
+  struct Inner {
+void f() {
+  T *var;
+}
+  };
+};
+}


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

[PATCH] D67509: [CUDA][HIP] Diagnose defaulted constructor only if it is used

2019-09-12 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.

Clang infers defaulted ctor as `__device__ __host__` and virtual dtor as 
`__host__`.

It diagnose the following code in device compilation as B() references ~A() 
implicitly.

  struct A {  virtual ~A(); };
  struct B: public A { B(); };
  B::B() = default;

Clang should not diagnose the above code since B() may be used in host function 
only.
Clang should diagnose only if B() is used in device function or kernel.

This patch fixes that by assuming defaulted class member as not always emitted. 
Therefore
a definition of defaulted special member function does not trigger a 
diagnostic, unless
the defaulted special member function is really called.


https://reviews.llvm.org/D67509

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


Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only 
-fcuda-is-device -verify %s
+
+#define __host__ __attribute__((host))
+#define __device__ __attribute__((device))
+
+struct Base {
+  virtual ~Base(); // expected-note {{'~Base' declared here}}
+};
+
+struct Unused : public Base {
+  Unused();
+};
+
+// Unused defaulted constructor should not cause diagnostics.
+Unused::Unused() = default;
+
+struct Used : public Base // expected-note {{'~Used' declared here}}
+{
+  Used();
+};
+
+Used::Used() = default; // expected-error {{reference to __host__ function 
'~Base' in __host__ __device__ function}}
+
+__device__ void f() {
+  Used x; // expected-error {{reference to __host__ function '~Used' in 
__device__ function}}
+  // expected-note@-1 {{called by 'f'}}
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -611,8 +611,12 @@
   // emitted, because (say) the definition could include "inline".
   FunctionDecl *Def = FD->getDefinition();
 
+  CXXMethodDecl *MD = dyn_cast_or_null(FD);
+
   if (Def &&
-  
!isDiscardableGVALinkage(S.getASTContext().GetGVALinkageForFunction(Def)))
+  !isDiscardableGVALinkage(
+  S.getASTContext().GetGVALinkageForFunction(Def)) &&
+  !(MD && MD->isDefaulted()))
 return true;
 
   // Otherwise, the function is known-emitted if it's in our set of


Index: test/SemaCUDA/default-ctor.cu
===
--- /dev/null
+++ test/SemaCUDA/default-ctor.cu
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
+
+#define __host__ __attribute__((host))
+#define __device__ __attribute__((device))
+
+struct Base {
+  virtual ~Base(); // expected-note {{'~Base' declared here}}
+};
+
+struct Unused : public Base {
+  Unused();
+};
+
+// Unused defaulted constructor should not cause diagnostics.
+Unused::Unused() = default;
+
+struct Used : public Base // expected-note {{'~Used' declared here}}
+{
+  Used();
+};
+
+Used::Used() = default; // expected-error {{reference to __host__ function '~Base' in __host__ __device__ function}}
+
+__device__ void f() {
+  Used x; // expected-error {{reference to __host__ function '~Used' in __device__ function}}
+  // expected-note@-1 {{called by 'f'}}
+}
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -611,8 +611,12 @@
   // emitted, because (say) the definition could include "inline".
   FunctionDecl *Def = FD->getDefinition();
 
+  CXXMethodDecl *MD = dyn_cast_or_null(FD);
+
   if (Def &&
-  !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageForFunction(Def)))
+  !isDiscardableGVALinkage(
+  S.getASTContext().GetGVALinkageForFunction(Def)) &&
+  !(MD && MD->isDefaulted()))
 return true;
 
   // Otherwise, the function is known-emitted if it's in our set of
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

2019-09-12 Thread Reid Kleckner via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371753: [MS] Warn when shadowing template parameters under 
-fms-compatibility (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67463?vs=219780&id=219956#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67463

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
  cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp


Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -849,15 +849,14 @@
 void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
-  // Microsoft Visual C++ permits template parameters to be shadowed.
-  if (getLangOpts().MicrosoftExt)
-return;
-
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
-  Diag(Loc, diag::err_template_param_shadow)
-<< cast(PrevDecl)->getDeclName();
+  //
+  // Make this a warning when MSVC compatibility is requested.
+  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
+ : diag::err_template_param_shadow;
+  Diag(Loc, DiagId) << cast(PrevDecl)->getDeclName();
   Diag(PrevDecl->getLocation(), diag::note_template_param_here);
 }
 
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4000,6 +4000,8 @@
 // C++ Template Declarations
 def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
+def ext_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Text>, InGroup;
 def note_template_param_here : Note<"template parameter is declared here">;
 def warn_template_export_unsupported : Warning<
   "exported templates are unsupported">;
Index: cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp
+++ cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -366,3 +366,21 @@
 int S::fn() { return 0; } // expected-warning {{is missing exception 
specification}}
 }
 
+namespace PR43265 {
+template  // expected-note {{template parameter is declared here}}
+struct Foo {
+  static const int N = 42; // expected-warning {{declaration of 'N' shadows 
template parameter}}
+};
+}
+
+namespace Inner_Outer_same_template_param_name {
+template  // expected-note {{template parameter is declared here}}
+struct Outmost {
+  template  // expected-warning {{declaration of 'T' shadows 
template parameter}}
+  struct Inner {
+void f() {
+  T *var;
+}
+  };
+};
+}
Index: cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
===
--- cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
+++ cfe/trunk/test/Parser/DelayedTemplateParsing.cpp
@@ -48,22 +48,6 @@
 
   
 
-namespace Inner_Outer_same_template_param_name {  
-
-template 
-class Outmost {
-public:
-template 
-class Inner {
-public:
-void f() {
-T* var;
-}
-   };
-};
-
-}
-
 
 namespace PR11931 {
 


Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -849,15 +849,14 @@
 void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) {
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
-  // Microsoft Visual C++ permits template parameters to be shadowed.
-  if (getLangOpts().MicrosoftExt)
-return;
-
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
-  Diag(Loc, diag::err_template_param_shadow)
-<< cast(PrevDecl)->getDeclName();
+  //
+  // Make this a warning when MSVC compatibility is requested.
+  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
+ : diag::err_template_param_shadow;
+  Diag(Loc, DiagId) << cast(PrevDecl)->getDeclName();
   Diag(PrevDecl->getLocation(), diag::note_template_param_here);
 }
 
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSema

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Conrad Poelman via Phabricator via cfe-commits 
poelmanc updated this revision to Diff 219957.
poelmanc added a comment.

Thanks for the stressing test cases. I reverted to your original test case with 
a single >, added a separate one with a single <, and expanded the final case 
to have a non-balanced number of > and <. I added all your new cases, with 
variations for non-fixed (multiple typedef) and fixed (single typedef) examples.

To make the braces example pass, we now only abandon the fix when encountering 
a non-nested open brace.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
@@ -182,4 +182,53 @@
 class E : public C {
   void f() override { super::f(); }
 };
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
 }
+
+template 
+struct S {};
+
+typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+
+typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+
+typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S3_t = S<(0 > 0 && (3 > 1) && (1 < 1)), int>;
+
+template 
+struct Q {};
+
+constexpr bool b[1] = {true};
+
+typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q Q_t, *Q_p;
+
+typedef Q Q2_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q2_t = Q;
+
+struct T {
+  constexpr T(bool) {}
+
+  static constexpr bool b = true;
+};
+
+typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+
+typedef Q Q3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -40,24 +40,54 @@
 
   Token Tok;
   int ParenLevel = 0;
+  int BraceLevel = 0;
+  int AngleBracketLevel = 0;
+  int SquareBracketLevel = 0;
   bool FoundTypedef = false;
 
   while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) {
 switch (Tok.getKind()) {
 case tok::l_brace:
+  if (ParenLevel == 0 && SquareBracketLevel == 0 && AngleBracketLevel == 0) {
+// At top level, this might be the `typedef struct {...} T;` case.
+// Inside parens, square brackets, or angle brackets it's not.
+return false;
+  }
+  BraceLevel++;
+  break;
 case tok::r_brace:
-  // This might be the `typedef struct {...} T;` case.
-  return false;
+  BraceLevel--;
+  break;
 case tok::l_paren:
   ParenLevel++;
   break;
 case tok::r_paren:
   ParenLevel--;
   break;
+case tok::l_square:
+  SquareBracketLevel++;
+  break;
+case tok::r_square:
+  SquareBracketLevel--;
+  break;
+case tok::less:
+  // If not nested, treat as opening angle bracket.
+  if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0)
+AngleBracketLevel++;
+  break;
+case tok::greater:
+  // Per C++ 17 Draft N4659, Section 17.2/3
+  //   https://timsong-cpp.github.io/cppwp/n4659/temp.names#3:
+  // "When parsing a template-argument-list, the first non-nested > is
+  // taken as the ending delimiter rather than a greater-than operator."
+  // If not nested, treat as closing angle bracket.
+  if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0)
+AngleBracketLevel--;
+  break;
 case tok::comma:
-  if (ParenLevel == 0) {
-// If there is comma and we are not between open parenthesis then it is
-// two or more declarations in this chain.
+  if (ParenLevel == 0 && SquareBracketLevel == 0 && BraceLevel == 0 &&
+  AngleBracketLevel == 0) {
+// If there is comma not nested then it is two or more declarations in this chain.
 return false;
   }
   break;
@@ -88,8 +118,7 @@
   if (StartLoc.isMacroID() && IgnoreMacros)
 return;
 
-  auto Diag =
-

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a subscriber: jkorous.
arphaman added a comment.

In D67091#1667821 , @kousikk wrote:

> Sorry about the delay on this - I was OOO (back now).
>
> 1. I added tests.
> 2. I couldn't add isDirectory() check to createFile() since that resulted in 
> failures to normal scenarios where it was previously passing.
>
>   PTAL!


I was unable to reproduce the failures when I moved the check to `createFile`. 
What kind of failures did you see? Did you move it right to the start of the 
function? I would recommend rebasing the patch and retrying.

Also, could you please rename `headerwithdirname.cpp` to 
`headerwithdirname_input.cpp` when copying it to ensure FileCheck can match the 
filename without matching the temporary path (see 
https://reviews.llvm.org/D67379 for test fixes that @jkorous applied).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091



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

r371756 - [analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

2019-09-12 Thread Kristof Umann via cfe-commits 
Author: szelethus
Date: Thu Sep 12 11:53:48 2019
New Revision: 371756

URL: http://llvm.org/viewvc/llvm-project?rev=371756&view=rev
Log:
[analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

Short and sweet. Whenever I use -analyzer-list-enabled-checkers, I'm only
interested about the configuration, not about the analysis.

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

Modified:
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/test/Analysis/analyzer-enabled-checkers.c

Modified: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp?rev=371756&r1=371755&r2=371756&view=diff
==
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp (original)
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp Thu Sep 12 
11:53:48 2019
@@ -270,6 +270,7 @@ bool ExecuteCompilerInvocation(CompilerI
   AnOpts,
   Clang->getDiagnostics(),
   Clang->getLangOpts());
+return true;
   }
 
   // Honor -analyzer-config-help.

Modified: cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-enabled-checkers.c?rev=371756&r1=371755&r2=371756&view=diff
==
--- cfe/trunk/test/Analysis/analyzer-enabled-checkers.c (original)
+++ cfe/trunk/test/Analysis/analyzer-enabled-checkers.c Thu Sep 12 11:53:48 2019
@@ -1,20 +1,55 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core -analyzer-list-enabled-checkers > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang --analyze %s \
+// RUN:   -Xclang -triple -Xclang x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-list-enabled-checkers \
+// RUN:   -Xclang -analyzer-display-progress \
+// RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
+// RUN:   --implicit-check-not=\.
 
-// CHECK: OVERVIEW: Clang Static Analyzer Enabled Checkers List
-// CHECK: core.CallAndMessage
-// CHECK: core.DivideZero
-// CHECK: core.DynamicTypePropagation
-// CHECK: core.NonNullParamChecker
-// CHECK: core.NullDereference
-// CHECK: core.StackAddressEscape
-// CHECK: core.UndefinedBinaryOperatorResult
-// CHECK: core.VLASize
-// CHECK: core.builtin.BuiltinFunctions
-// CHECK: core.builtin.NoReturnFunctions
-// CHECK: core.uninitialized.ArraySubscript
-// CHECK: core.uninitialized.Assign
-// CHECK: core.uninitialized.Branch
-// CHECK: core.uninitialized.CapturedBlockVariable
-// CHECK: core.uninitialized.UndefReturn
+// CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
+// CHECK-EMPTY:
+// CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: apiModeling.TrustNonnull
+// CHECK-NEXT: apiModeling.llvm.CastValue
+// CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.CallAndMessage
+// CHECK-NEXT: core.DivideZero
+// CHECK-NEXT: core.DynamicTypePropagation
+// CHECK-NEXT: core.NonNullParamChecker
+// CHECK-NEXT: core.NonnilStringConstants
+// CHECK-NEXT: core.NullDereference
+// CHECK-NEXT: core.StackAddrEscapeBase
+// CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.UndefinedBinaryOperatorResult
+// CHECK-NEXT: core.VLASize
+// CHECK-NEXT: core.builtin.BuiltinFunctions
+// CHECK-NEXT: core.builtin.NoReturnFunctions
+// CHECK-NEXT: core.uninitialized.ArraySubscript
+// CHECK-NEXT: core.uninitialized.Assign
+// CHECK-NEXT: core.uninitialized.Branch
+// CHECK-NEXT: core.uninitialized.CapturedBlockVariable
+// CHECK-NEXT: core.uninitialized.UndefReturn
+// CHECK-NEXT: deadcode.DeadStores
+// CHECK-NEXT: nullability.NullabilityBase
+// CHECK-NEXT: nullability.NullPassedToNonnull
+// CHECK-NEXT: nullability.NullReturnedFromNonnull
+// CHECK-NEXT: security.insecureAPI.SecuritySyntaxChecker
+// CHECK-NEXT: security.insecureAPI.UncheckedReturn
+// CHECK-NEXT: security.insecureAPI.getpw
+// CHECK-NEXT: security.insecureAPI.gets
+// CHECK-NEXT: security.insecureAPI.mkstemp
+// CHECK-NEXT: security.insecureAPI.mktemp
+// CHECK-NEXT: security.insecureAPI.vfork
+// CHECK-NEXT: unix.API
+// CHECK-NEXT: unix.cstring.CStringModeling
+// CHECK-NEXT: unix.DynamicMemoryModeling
+// CHECK-NEXT: unix.Malloc
+// CHECK-NEXT: unix.MallocSizeof
+// CHECK-NEXT: unix.MismatchedDeallocator
+// CHECK-NEXT: unix.Vfork
+// CHECK-NEXT: unix.cstring.BadSizeArg
+// CHECK-NEXT: unix.cstring.NullArg
 
+int main() {
+  int i;
+  (void)(10 / i);
+}


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

[PATCH] D66981: Fix driver tests when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is `ON`

2019-09-12 Thread Sergej Jaskiewicz via Phabricator via cfe-commits 
broadwaylamb added a comment.

In D66981#1651897 , @phosek wrote:

> LGTM


Can you please commit this? I don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66981



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

[PATCH] D66714: [analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

2019-09-12 Thread Kristóf Umann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371756: [analyzer] Don't run the analyzer for 
-analyzer-list-enabled-checkers (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66714?vs=217039&id=219961#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66714

Files:
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/test/Analysis/analyzer-enabled-checkers.c


Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -270,6 +270,7 @@
   AnOpts,
   Clang->getDiagnostics(),
   Clang->getLangOpts());
+return true;
   }
 
   // Honor -analyzer-config-help.
Index: cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
===
--- cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
+++ cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
@@ -1,20 +1,55 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core -analyzer-list-enabled-checkers > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang --analyze %s \
+// RUN:   -Xclang -triple -Xclang x86_64-pc-linux-gnu \
+// RUN:   -Xclang -analyzer-list-enabled-checkers \
+// RUN:   -Xclang -analyzer-display-progress \
+// RUN:   2>&1 | FileCheck %s --implicit-check-not=ANALYZE \
+// RUN:   --implicit-check-not=\.
 
-// CHECK: OVERVIEW: Clang Static Analyzer Enabled Checkers List
-// CHECK: core.CallAndMessage
-// CHECK: core.DivideZero
-// CHECK: core.DynamicTypePropagation
-// CHECK: core.NonNullParamChecker
-// CHECK: core.NullDereference
-// CHECK: core.StackAddressEscape
-// CHECK: core.UndefinedBinaryOperatorResult
-// CHECK: core.VLASize
-// CHECK: core.builtin.BuiltinFunctions
-// CHECK: core.builtin.NoReturnFunctions
-// CHECK: core.uninitialized.ArraySubscript
-// CHECK: core.uninitialized.Assign
-// CHECK: core.uninitialized.Branch
-// CHECK: core.uninitialized.CapturedBlockVariable
-// CHECK: core.uninitialized.UndefReturn
+// CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
+// CHECK-EMPTY:
+// CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: apiModeling.TrustNonnull
+// CHECK-NEXT: apiModeling.llvm.CastValue
+// CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.CallAndMessage
+// CHECK-NEXT: core.DivideZero
+// CHECK-NEXT: core.DynamicTypePropagation
+// CHECK-NEXT: core.NonNullParamChecker
+// CHECK-NEXT: core.NonnilStringConstants
+// CHECK-NEXT: core.NullDereference
+// CHECK-NEXT: core.StackAddrEscapeBase
+// CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.UndefinedBinaryOperatorResult
+// CHECK-NEXT: core.VLASize
+// CHECK-NEXT: core.builtin.BuiltinFunctions
+// CHECK-NEXT: core.builtin.NoReturnFunctions
+// CHECK-NEXT: core.uninitialized.ArraySubscript
+// CHECK-NEXT: core.uninitialized.Assign
+// CHECK-NEXT: core.uninitialized.Branch
+// CHECK-NEXT: core.uninitialized.CapturedBlockVariable
+// CHECK-NEXT: core.uninitialized.UndefReturn
+// CHECK-NEXT: deadcode.DeadStores
+// CHECK-NEXT: nullability.NullabilityBase
+// CHECK-NEXT: nullability.NullPassedToNonnull
+// CHECK-NEXT: nullability.NullReturnedFromNonnull
+// CHECK-NEXT: security.insecureAPI.SecuritySyntaxChecker
+// CHECK-NEXT: security.insecureAPI.UncheckedReturn
+// CHECK-NEXT: security.insecureAPI.getpw
+// CHECK-NEXT: security.insecureAPI.gets
+// CHECK-NEXT: security.insecureAPI.mkstemp
+// CHECK-NEXT: security.insecureAPI.mktemp
+// CHECK-NEXT: security.insecureAPI.vfork
+// CHECK-NEXT: unix.API
+// CHECK-NEXT: unix.cstring.CStringModeling
+// CHECK-NEXT: unix.DynamicMemoryModeling
+// CHECK-NEXT: unix.Malloc
+// CHECK-NEXT: unix.MallocSizeof
+// CHECK-NEXT: unix.MismatchedDeallocator
+// CHECK-NEXT: unix.Vfork
+// CHECK-NEXT: unix.cstring.BadSizeArg
+// CHECK-NEXT: unix.cstring.NullArg
 
+int main() {
+  int i;
+  (void)(10 / i);
+}


Index: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -270,6 +270,7 @@
   AnOpts,
   Clang->getDiagnostics(),
   Clang->getLangOpts());
+return true;
   }
 
   // Honor -analyzer-config-help.
Index: cfe/trunk/test/Analysis/analyzer-enabled-checkers.c
===
--- cfe/trunk/test/Analys

r371759 - NFC, add missing cl::cat option category to clang-scan-deps options to ensure they show up in -help

2019-09-12 Thread Alex Lorenz via cfe-commits 
Author: arphaman
Date: Thu Sep 12 12:00:32 2019
New Revision: 371759

URL: http://llvm.org/viewvc/llvm-project?rev=371759&view=rev
Log:
NFC, add missing cl::cat option category to clang-scan-deps options to ensure 
they show up in -help

Modified:
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Modified: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp?rev=371759&r1=371758&r2=371759&view=diff
==
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp (original)
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp Thu Sep 12 12:00:32 2019
@@ -150,13 +150,14 @@ static llvm::cl::opt ScanM
 clEnumValN(ScanningMode::CanonicalPreprocessing, "preprocess",
"The set of dependencies is computed by preprocessing the "
"unmodified source files")),
-llvm::cl::init(ScanningMode::MinimizedSourcePreprocessing));
+llvm::cl::init(ScanningMode::MinimizedSourcePreprocessing),
+llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
   "all concurrent threads)"),
-   llvm::cl::init(0));
+   llvm::cl::init(0), llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt
 CompilationDB("compilation-database",


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

r371760 - [analyzer][NFC] Fix inconsistent references to checkers as "checks"

2019-09-12 Thread Kristof Umann via cfe-commits 
Author: szelethus
Date: Thu Sep 12 12:09:24 2019
New Revision: 371760

URL: http://llvm.org/viewvc/llvm-project?rev=371760&view=rev
Log:
[analyzer][NFC] Fix inconsistent references to checkers as "checks"

Traditionally, clang-tidy uses the term check, and the analyzer uses checker,
but in the very early years, this wasn't the case, and code originating from the
early 2010's still incorrectly refer to checkers as checks.

This patch attempts to hunt down most of these, aiming to refer to checkers as
checkers, but preserve references to callback functions (like checkPreCall) as
checks.

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

Modified:
cfe/trunk/include/clang/Analysis/PathDiagnostic.h
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/lib/Analysis/PathDiagnostic.cpp
cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Checker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Modified: cfe/trunk/include/clang/Analysis/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathDiagnostic.h?rev=371760&r1=371759&r2=371760&view=diff
==
--- cfe/trunk/include/clang/Analysis/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/Analysis/PathDiagnostic.h Thu Sep 12 12:09:24 2019
@@ -725,7 +725,7 @@ using FilesToLineNumsMap = std::maphttp://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=371760&r1=371759&r2=371760&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu Sep 12 
12:09:24 2019
@@ -163,41 +163,15 @@ class AnalyzerOptions : public RefCounte
 public:
   using ConfigTable = llvm::StringMap;
 
+  /// Retrieves the list of checkers generated from Checkers.td. This doesn't
+  /// contain statically linked but non-generated checkers and plugin checkers!
   static std::vector
-  getRegisteredCheckers(bool IncludeExperimental = false) {
-static const StringRef StaticAnalyzerChecks[] = {
-#define GET_CHECKERS
-#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI, IS_HIDDEN) FULLNAME,
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
-#undef CHECKER
-#undef GET_CHECKERS
-};
-std::vector Checkers;
-for (StringRef CheckerName : StaticAnalyzerChecks) {
-  if (!CheckerName.startswith("debug.") &&
-  (IncludeExperimental || !CheckerName.startswith("alpha.")))
-Checkers.push_back(CheckerName);
-}
-return Checkers;
-  }
+  getRegisteredCheckers(bool IncludeExperimental = false);
 
+  /// Retrieves the list of packages generated from Checkers.td. This doesn't
+  /// contain statically linked but non-generated packages and plugin packages!
   static std::vector
-  getRegisteredPackages(bool IncludeExperimental = false) {
-static const StringRef StaticAnalyzerPackages[] = {
-#define GET_PACKAGES
-#define PACKAGE(FULLNAME) FULLNAME,
-#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
-#undef PACKAGE
-#undef GET_PACKAGES
-};
-std::vector Packages;
-for (StringRef Packa

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

2019-09-12 Thread Kristóf Umann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371760: [analyzer][NFC] Fix inconsistent references to 
checkers as "checks" (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67140?vs=218583&id=219968#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67140

Files:
  cfe/trunk/include/clang/Analysis/PathDiagnostic.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  cfe/trunk/lib/Analysis/PathDiagnostic.cpp
  cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/Checker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1312,7 +1312,7 @@
 generateDiagnosticForBasicReport(const BasicBugReport *R) {
   const BugType &BT = R->getBugType();
   return std::make_unique(
-  BT.getCheckName(), R->getDeclWithIssue(), BT.getName(),
+  BT.getCheckerName(), R->getDeclWithIssue(), BT.getDescription(),
   R->getDescription(), R->getShortDescription(/*UseFallback=*/false),
   BT.getCategory(), R->getUniqueingLocation(), R->getUniqueingDecl(),
   std::make_unique());
@@ -1323,7 +1323,7 @@
  const SourceManager &SM) {
   const BugType &BT = R->getBugType();
   return std::make_unique(
-  BT.getCheckName(), R->getDeclWithIssue(), BT.getName(),
+  BT.getCheckerName(), R->getDeclWithIssue(), BT.getDescription(),
   R->getDescription(), R->getShortDescription(/*UseFallback=*/false),
   BT.getCategory(), R->getUniqueingLocation(), R->getUniqueingDecl(),
   findExecutedLines(SM, R->getErrorNode()));
@@ -3235,12 +3235,12 @@
   PathDiagnosticLocation Loc,
   ArrayRef Ranges,
   ArrayRef Fixits) {
-  EmitBasicReport(DeclWithIssue, Checker->getCheckName(), Name, Category, Str,
+  EmitBasicReport(DeclWithIssue, Checker->getCheckerName(), Name, Category, Str,
   Loc, Ranges, Fixits);
 }
 
 void BugReporter::EmitBasicReport(const Decl *DeclWithIssue,
-  CheckName CheckName,
+  CheckerNameRef CheckName,
   StringRef name, StringRef category,
   StringRef str, PathDiagnosticLocation Loc,
   ArrayRef Ranges,
@@ -3256,8 +3256,8 @@
   emitReport(std::move(R));
 }
 
-BugType *BugReporter::getBugTypeForName(CheckName CheckName, StringRef name,
-StringRef category) {
+BugType *BugReporter::getBugTypeForName(CheckerNameRef CheckName,
+StringRef name, StringRef category) {
   SmallString<136> fullDesc;
   llvm::raw_svector_ostream(fullDesc) << CheckName.getName() << ":" << name
   << ":" << category;
Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
==

[PATCH] D67509: [CUDA][HIP] Diagnose defaulted constructor only if it is used

2019-09-12 Thread Artem Belevich via Phabricator via cfe-commits 
tra added a comment.

Example of the actual error produced by clang: https://godbolt.org/z/Dl1FfC

Ugh. Another corner case of the way we're dealing with implicit `__host__ 
__device__` functions. :-(
LGTM for postponing the error until actual use.




Comment at: test/SemaCUDA/default-ctor.cu:1
+// RUN: %clang_cc1 -std=c++11 -triple nvptx64-nvidia-cuda -fsyntax-only 
-fcuda-is-device -verify %s
+

It would be good to add host-side compilation, too. 



Comment at: test/SemaCUDA/default-ctor.cu:4
+#define __host__ __attribute__((host))
+#define __device__ __attribute__((device))
+

Use `#include "Inputs/cuda.h"` instead.


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

https://reviews.llvm.org/D67509



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

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219975.
kousikk added a comment.

- Add validation inside the createFile() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/foodir
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/headerwithdirname.cpp


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs 
DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -193,6 +193,8 @@
 llvm::ErrorOr>
 createFile(const CachedFileSystemEntry *Entry,
ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) {
+  if (Entry->isDirectory())
+return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
@@ -215,8 +217,9 @@
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
   // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
+  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename)) {
 return createFile(Entry, PPSkipMappings);
+  }
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -56,6 +56,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
==

[PATCH] D67515: [Sema][Typo Correction] Fix potential infite loop on ambiguity checks

2019-09-12 Thread David Goldman via Phabricator via cfe-commits 
dgoldman created this revision.
dgoldman added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes a bug introduced in D62648 , where 
Clang could infinite loop
if it became stuck on a single TypoCorrection when it was supposed to
be testing ambiguous corrections. Although not a common case, it could
happen if there are multiple possible corrections with the same edit
distance.

The fix is simply to wipe the TypoExpr from the `TransformCache` so that
the call to `TransformTypoExpr` doesn't use the `CachedEntry`.


Repository:
  rC Clang

https://reviews.llvm.org/D67515

Files:
  lib/Sema/SemaExprCXX.cpp
  test/Sema/typo-correction-ambiguity.cpp


Index: test/Sema/typo-correction-ambiguity.cpp
===
--- /dev/null
+++ test/Sema/typo-correction-ambiguity.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in namespaces:
+// - no typos are diagnosed when an expression has ambiguous (multiple) 
corrections
+// - proper iteration through multiple potentially ambiguous corrections
+
+namespace AmibguousCorrection
+{
+  void method_Bar();
+  void method_Foo();
+  void method_Zoo();
+};
+
+void testAmbiguousNoSuggestions()
+{
+  AmibguousCorrection::method_Ace(); // expected-error {{no member named 
'method_Ace' in namespace 'AmibguousCorrection'}}
+}
+
+namespace MultipleCorrectionsButNotAmbiguous
+{
+  int PrefixType_Name(int value);  // expected-note {{'PrefixType_Name' 
declared here}}
+  int PrefixType_MIN();
+  int PrefixType_MAX();
+};
+
+int testDirectAmbiguousCorrection() {
+  int val = MultipleCorrectionsButNotAmbiguous::PrefixType_Enum(0);  // 
expected-error {{no member named 'PrefixType_Enum' in namespace 
'MultipleCorrectionsButNotAmbiguous'; did you mean 'PrefixType_Name'?}}
+  return val;
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7755,6 +7755,10 @@
 TypoCorrection TC = 
SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection();
 TypoCorrection Next;
 do {
+  // Fetch the next correction by erasing the typo from the cache and 
calling
+  // `TryTransform` which will iterate through corrections in
+  // `TransformTypoExpr`.
+  TransformCache.erase(TE);
   ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(E), 
IsAmbiguous);
 
   if (!AmbigRes.isInvalid() || IsAmbiguous) {


Index: test/Sema/typo-correction-ambiguity.cpp
===
--- /dev/null
+++ test/Sema/typo-correction-ambiguity.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in namespaces:
+// - no typos are diagnosed when an expression has ambiguous (multiple) corrections
+// - proper iteration through multiple potentially ambiguous corrections
+
+namespace AmibguousCorrection
+{
+  void method_Bar();
+  void method_Foo();
+  void method_Zoo();
+};
+
+void testAmbiguousNoSuggestions()
+{
+  AmibguousCorrection::method_Ace(); // expected-error {{no member named 'method_Ace' in namespace 'AmibguousCorrection'}}
+}
+
+namespace MultipleCorrectionsButNotAmbiguous
+{
+  int PrefixType_Name(int value);  // expected-note {{'PrefixType_Name' declared here}}
+  int PrefixType_MIN();
+  int PrefixType_MAX();
+};
+
+int testDirectAmbiguousCorrection() {
+  int val = MultipleCorrectionsButNotAmbiguous::PrefixType_Enum(0);  // expected-error {{no member named 'PrefixType_Enum' in namespace 'MultipleCorrectionsButNotAmbiguous'; did you mean 'PrefixType_Name'?}}
+  return val;
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7755,6 +7755,10 @@
 TypoCorrection TC = SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection();
 TypoCorrection Next;
 do {
+  // Fetch the next correction by erasing the typo from the cache and calling
+  // `TryTransform` which will iterate through corrections in
+  // `TransformTypoExpr`.
+  TransformCache.erase(TE);
   ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(E), IsAmbiguous);
 
   if (!AmbigRes.isInvalid() || IsAmbiguous) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67091: Fix for headers having the same name as a directory

2019-09-12 Thread Kousik Kumar via Phabricator via cfe-commits 
kousikk updated this revision to Diff 219976.
kousikk added a comment.

- Add validation inside the createFile() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/Inputs/foodir
  clang/test/ClangScanDeps/Inputs/headerwithdirname.json
  clang/test/ClangScanDeps/headerwithdirname.cpp


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs 
DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -193,6 +193,8 @@
 llvm::ErrorOr>
 createFile(const CachedFileSystemEntry *Entry,
ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) {
+  if (Entry->isDirectory())
+return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
   llvm::ErrorOr Contents = Entry->getContents();
   if (!Contents)
 return Contents.getError();
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -56,6 +56,11 @@
   /// \returns True if the entry is valid.
   bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); }
 
+  /// \returns True if the current entry points to a directory.
+  bool isDirectory() const {
+return MaybeStat && MaybeStat->isDirectory();
+  }
+
   /// \returns The error or the file's contents.
   llvm::ErrorOr getContents() const {
 if (!MaybeStat)


Index: clang/test/ClangScanDeps/headerwithdirname.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/headerwithdirname.cpp
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.dir/foodir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: mkdir -p %t.dir/foodir
+// RUN: cp %s %t.dir/headerwithdirname_input.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/foodir %t.dir/Inputs/foodir
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/headerwithdirname.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#include 
+
+// CHECK: headerwithdirname_input.o
+// CHECK-NEXT: headerwithdirname_input.cpp
+// CHECK-NEXT: Inputs{{/|\\}}foodir
Index: clang/test/ClangScanDeps/Inputs/headerwithdirname.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/headerwithdirname.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c -IDIR -IDIR/foodir -IInputs DIR/headerwithdirname_input.cpp",
+  "file": "DIR/headerwithdirname_input.cpp"
+}
+]
Index: clang/test/ClangScanDeps/Inputs/foodir
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/foodir
@@ -0,0 +1 @@
+// A C++ header with same name as that of a directory in the include path.
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFi

[PATCH] D67091: Fix for headers having the same name as a directory

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

> I was unable to reproduce the failures when I moved the check to createFile. 
> What kind of failures did you see? Did you move it right to the start of the 
> function? I would recommend rebasing the patch and retrying.

I saw an assertion error - but I realize now that it was because it was not at 
the top of the function. I had put it after `Entry->getContents()` which was 
throwing an assertion. I now realize that for `getContents()` to work, the 
entry needs to be a file. Thanks for correcting me!

> Also, could you please rename headerwithdirname.cpp to 
> headerwithdirname_input.cpp when copying it to ensure FileCheck can match the 
> filename without matching the temporary path (see 
> https://reviews.llvm.org/D67379 for test fixes that @jkorous applied).

Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67091



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

[PATCH] D67515: [Sema][Typo Correction] Fix potential infite loop on ambiguity checks

2019-09-12 Thread David Goldman via Phabricator via cfe-commits 
dgoldman updated this revision to Diff 219978.
dgoldman added a comment.

- Fix method name in test


Repository:
  rC Clang

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

https://reviews.llvm.org/D67515

Files:
  lib/Sema/SemaExprCXX.cpp
  test/Sema/typo-correction-ambiguity.cpp


Index: test/Sema/typo-correction-ambiguity.cpp
===
--- /dev/null
+++ test/Sema/typo-correction-ambiguity.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in namespaces:
+// - no typos are diagnosed when an expression has ambiguous (multiple) 
corrections
+// - proper iteration through multiple potentially ambiguous corrections
+
+namespace AmibguousCorrection
+{
+  void method_Bar();
+  void method_Foo();
+  void method_Zoo();
+};
+
+void testAmbiguousNoSuggestions()
+{
+  AmibguousCorrection::method_Ace(); // expected-error {{no member named 
'method_Ace' in namespace 'AmibguousCorrection'}}
+}
+
+namespace MultipleCorrectionsButNotAmbiguous
+{
+  int PrefixType_Name(int value);  // expected-note {{'PrefixType_Name' 
declared here}}
+  int PrefixType_MIN();
+  int PrefixType_MAX();
+};
+
+int testMultipleCorrectionsButNotAmbiguous() {
+  int val = MultipleCorrectionsButNotAmbiguous::PrefixType_Enum(0);  // 
expected-error {{no member named 'PrefixType_Enum' in namespace 
'MultipleCorrectionsButNotAmbiguous'; did you mean 'PrefixType_Name'?}}
+  return val;
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7755,6 +7755,10 @@
 TypoCorrection TC = 
SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection();
 TypoCorrection Next;
 do {
+  // Fetch the next correction by erasing the typo from the cache and 
calling
+  // `TryTransform` which will iterate through corrections in
+  // `TransformTypoExpr`.
+  TransformCache.erase(TE);
   ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(E), 
IsAmbiguous);
 
   if (!AmbigRes.isInvalid() || IsAmbiguous) {


Index: test/Sema/typo-correction-ambiguity.cpp
===
--- /dev/null
+++ test/Sema/typo-correction-ambiguity.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check the following typo correction behavior in namespaces:
+// - no typos are diagnosed when an expression has ambiguous (multiple) corrections
+// - proper iteration through multiple potentially ambiguous corrections
+
+namespace AmibguousCorrection
+{
+  void method_Bar();
+  void method_Foo();
+  void method_Zoo();
+};
+
+void testAmbiguousNoSuggestions()
+{
+  AmibguousCorrection::method_Ace(); // expected-error {{no member named 'method_Ace' in namespace 'AmibguousCorrection'}}
+}
+
+namespace MultipleCorrectionsButNotAmbiguous
+{
+  int PrefixType_Name(int value);  // expected-note {{'PrefixType_Name' declared here}}
+  int PrefixType_MIN();
+  int PrefixType_MAX();
+};
+
+int testMultipleCorrectionsButNotAmbiguous() {
+  int val = MultipleCorrectionsButNotAmbiguous::PrefixType_Enum(0);  // expected-error {{no member named 'PrefixType_Enum' in namespace 'MultipleCorrectionsButNotAmbiguous'; did you mean 'PrefixType_Name'?}}
+  return val;
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -7755,6 +7755,10 @@
 TypoCorrection TC = SemaRef.getTypoExprState(TE).Consumer->peekNextCorrection();
 TypoCorrection Next;
 do {
+  // Fetch the next correction by erasing the typo from the cache and calling
+  // `TryTransform` which will iterate through corrections in
+  // `TransformTypoExpr`.
+  TransformCache.erase(TE);
   ExprResult AmbigRes = CheckForRecursiveTypos(TryTransform(E), IsAmbiguous);
 
   if (!AmbigRes.isInvalid() || IsAmbiguous) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371765 - [CFG] Add dumps for CFGElement and CFGElementRef

2019-09-12 Thread Kristof Umann via cfe-commits 
Author: szelethus
Date: Thu Sep 12 12:52:34 2019
New Revision: 371765

URL: http://llvm.org/viewvc/llvm-project?rev=371765&view=rev
Log:
[CFG] Add dumps for CFGElement and CFGElementRef

Seems like we never had these, so here we go! I also did some refactoring as I
was chasing a bug unrelated to this revision.

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

Modified:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/lib/Analysis/CFG.cpp

Modified: cfe/trunk/include/clang/Analysis/CFG.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=371765&r1=371764&r2=371765&view=diff
==
--- cfe/trunk/include/clang/Analysis/CFG.h (original)
+++ cfe/trunk/include/clang/Analysis/CFG.h Thu Sep 12 12:52:34 2019
@@ -121,6 +121,12 @@ public:
 x |= Data1.getInt();
 return (Kind) x;
   }
+
+  void dumpToStream(llvm::raw_ostream &OS) const;
+
+  void dump() const {
+dumpToStream(llvm::errs());
+  }
 };
 
 class CFGStmt : public CFGElement {
@@ -650,8 +656,17 @@ class CFGBlock {
 }
 
 bool operator!=(ElementRefImpl Other) const { return !(*this == Other); }
-CFGElement operator*() { return (*Parent)[Index]; }
-CFGElementPtr operator->() { return &*(Parent->begin() + Index); }
+CFGElement operator*() const { return (*Parent)[Index]; }
+CFGElementPtr operator->() const { return &*(Parent->begin() + Index); }
+
+void dumpToStream(llvm::raw_ostream &OS) const {
+  OS << getIndexInBlock() + 1 << ": ";
+  (*this)->dumpToStream(OS);
+}
+
+void dump() const {
+  dumpToStream(llvm::errs());
+}
   };
 
   template  class ElementRefIterator {

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=371765&r1=371764&r2=371765&view=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Thu Sep 12 12:52:34 2019
@@ -4999,6 +4999,8 @@ class StmtPrinterHelper : public Printer
 public:
   StmtPrinterHelper(const CFG* cfg, const LangOptions &LO)
   : LangOpts(LO) {
+if (!cfg)
+  return;
 for (CFG::const_iterator I = cfg->begin(), E = cfg->end(); I != E; ++I ) {
   unsigned j = 1;
   for (CFGBlock::const_iterator BI = (*I)->begin(), BEnd = (*I)->end() ;
@@ -5321,9 +5323,21 @@ static void print_construction_context(r
 }
 
 static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
+   const CFGElement &E);
+
+void CFGElement::dumpToStream(llvm::raw_ostream &OS) const {
+  StmtPrinterHelper Helper(nullptr, {});
+  print_elem(OS, Helper, *this);
+}
+
+static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
const CFGElement &E) {
-  if (Optional CS = E.getAs()) {
-const Stmt *S = CS->getStmt();
+  switch (E.getKind()) {
+  case CFGElement::Kind::Statement:
+  case CFGElement::Kind::CXXRecordTypedCall:
+  case CFGElement::Kind::Constructor: {
+CFGStmt CS = E.castAs();
+const Stmt *S = CS.getStmt();
 assert(S != nullptr && "Expecting non-null Stmt");
 
 // special printing for statement-expressions.
@@ -5375,12 +5389,18 @@ static void print_elem(raw_ostream &OS,
 // Expressions need a newline.
 if (isa(S))
   OS << '\n';
-  } else if (Optional IE = E.getAs()) {
-print_initializer(OS, Helper, IE->getInitializer());
+
+break;
+  }
+
+  case CFGElement::Kind::Initializer:
+print_initializer(OS, Helper, E.castAs().getInitializer());
 OS << '\n';
-  } else if (Optional DE =
- E.getAs()) {
-const VarDecl *VD = DE->getVarDecl();
+break;
+
+  case CFGElement::Kind::AutomaticObjectDtor: {
+CFGAutomaticObjDtor DE = E.castAs();
+const VarDecl *VD = DE.getVarDecl();
 Helper.handleDecl(VD, OS);
 
 QualType T = VD->getType();
@@ -5390,53 +5410,75 @@ static void print_elem(raw_ostream &OS,
 OS << ".~";
 T.getUnqualifiedType().print(OS, PrintingPolicy(Helper.getLangOpts()));
 OS << "() (Implicit destructor)\n";
-  } else if (Optional DE = E.getAs()) {
-const VarDecl *VD = DE->getVarDecl();
-Helper.handleDecl(VD, OS);
+break;
+  }
 
+  case CFGElement::Kind::LifetimeEnds:
+Helper.handleDecl(E.castAs().getVarDecl(), OS);
 OS << " (Lifetime ends)\n";
-  } else if (Optional LE = E.getAs()) {
-const Stmt *LoopStmt = LE->getLoopStmt();
-OS << LoopStmt->getStmtClassName() << " (LoopExit)\n";
-  } else if (Optional SB = E.getAs()) {
+break;
+
+  case CFGElement::Kind::LoopExit:
+OS << E.castAs().getLoopStmt()->getStmtClassName() << " 
(LoopExit)\n";
+break;
+
+  case CFGElement::Kind::ScopeBegin:
 OS << "CFGScopeBegin(";
-if (const VarDecl *VD = SB->getVarDecl())
+if (const VarDecl *VD = E.castAs().getVarDecl())
   OS << VD->getQualifiedNameAsString();
 OS << ")\n";
-  } else if (

[PATCH] D66715: [CFG] Add dumps for CFGElement and CFGElementRef

2019-09-12 Thread Kristóf Umann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371765: [CFG] Add dumps for CFGElement and CFGElementRef 
(authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66715?vs=217040&id=219979#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66715

Files:
  cfe/trunk/include/clang/Analysis/CFG.h
  cfe/trunk/lib/Analysis/CFG.cpp

Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -4999,6 +4999,8 @@
 public:
   StmtPrinterHelper(const CFG* cfg, const LangOptions &LO)
   : LangOpts(LO) {
+if (!cfg)
+  return;
 for (CFG::const_iterator I = cfg->begin(), E = cfg->end(); I != E; ++I ) {
   unsigned j = 1;
   for (CFGBlock::const_iterator BI = (*I)->begin(), BEnd = (*I)->end() ;
@@ -5321,9 +5323,21 @@
 }
 
 static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
+   const CFGElement &E);
+
+void CFGElement::dumpToStream(llvm::raw_ostream &OS) const {
+  StmtPrinterHelper Helper(nullptr, {});
+  print_elem(OS, Helper, *this);
+}
+
+static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper,
const CFGElement &E) {
-  if (Optional CS = E.getAs()) {
-const Stmt *S = CS->getStmt();
+  switch (E.getKind()) {
+  case CFGElement::Kind::Statement:
+  case CFGElement::Kind::CXXRecordTypedCall:
+  case CFGElement::Kind::Constructor: {
+CFGStmt CS = E.castAs();
+const Stmt *S = CS.getStmt();
 assert(S != nullptr && "Expecting non-null Stmt");
 
 // special printing for statement-expressions.
@@ -5375,12 +5389,18 @@
 // Expressions need a newline.
 if (isa(S))
   OS << '\n';
-  } else if (Optional IE = E.getAs()) {
-print_initializer(OS, Helper, IE->getInitializer());
+
+break;
+  }
+
+  case CFGElement::Kind::Initializer:
+print_initializer(OS, Helper, E.castAs().getInitializer());
 OS << '\n';
-  } else if (Optional DE =
- E.getAs()) {
-const VarDecl *VD = DE->getVarDecl();
+break;
+
+  case CFGElement::Kind::AutomaticObjectDtor: {
+CFGAutomaticObjDtor DE = E.castAs();
+const VarDecl *VD = DE.getVarDecl();
 Helper.handleDecl(VD, OS);
 
 QualType T = VD->getType();
@@ -5390,53 +5410,75 @@
 OS << ".~";
 T.getUnqualifiedType().print(OS, PrintingPolicy(Helper.getLangOpts()));
 OS << "() (Implicit destructor)\n";
-  } else if (Optional DE = E.getAs()) {
-const VarDecl *VD = DE->getVarDecl();
-Helper.handleDecl(VD, OS);
+break;
+  }
 
+  case CFGElement::Kind::LifetimeEnds:
+Helper.handleDecl(E.castAs().getVarDecl(), OS);
 OS << " (Lifetime ends)\n";
-  } else if (Optional LE = E.getAs()) {
-const Stmt *LoopStmt = LE->getLoopStmt();
-OS << LoopStmt->getStmtClassName() << " (LoopExit)\n";
-  } else if (Optional SB = E.getAs()) {
+break;
+
+  case CFGElement::Kind::LoopExit:
+OS << E.castAs().getLoopStmt()->getStmtClassName() << " (LoopExit)\n";
+break;
+
+  case CFGElement::Kind::ScopeBegin:
 OS << "CFGScopeBegin(";
-if (const VarDecl *VD = SB->getVarDecl())
+if (const VarDecl *VD = E.castAs().getVarDecl())
   OS << VD->getQualifiedNameAsString();
 OS << ")\n";
-  } else if (Optional SE = E.getAs()) {
+break;
+
+  case CFGElement::Kind::ScopeEnd:
 OS << "CFGScopeEnd(";
-if (const VarDecl *VD = SE->getVarDecl())
+if (const VarDecl *VD = E.castAs().getVarDecl())
   OS << VD->getQualifiedNameAsString();
 OS << ")\n";
-  } else if (Optional NE = E.getAs()) {
+break;
+
+  case CFGElement::Kind::NewAllocator:
 OS << "CFGNewAllocator(";
-if (const CXXNewExpr *AllocExpr = NE->getAllocatorExpr())
+if (const CXXNewExpr *AllocExpr = E.castAs().getAllocatorExpr())
   AllocExpr->getType().print(OS, PrintingPolicy(Helper.getLangOpts()));
 OS << ")\n";
-  } else if (Optional DE = E.getAs()) {
-const CXXRecordDecl *RD = DE->getCXXRecordDecl();
+break;
+
+  case CFGElement::Kind::DeleteDtor: {
+CFGDeleteDtor DE = E.castAs();
+const CXXRecordDecl *RD = DE.getCXXRecordDecl();
 if (!RD)
   return;
 CXXDeleteExpr *DelExpr =
-const_cast(DE->getDeleteExpr());
+const_cast(DE.getDeleteExpr());
 Helper.handledStmt(cast(DelExpr->getArgument()), OS);
 OS << "->~" << RD->getName().str() << "()";
 OS << " (Implicit destructor)\n";
-  } else if (Optional BE = E.getAs()) {
-const CXXBaseSpecifier *BS = BE->getBaseSpecifier();
+break;
+  }
+
+  case CFGElement::Kind::BaseDtor: {
+const CXXBaseSpecifier *BS = E.castAs().getBaseSpecifier();
 OS << "~" << BS->getType()->getAsCXXRecordDecl()->getName() << "()";
 OS << " (Base object destructor)\n";
-  } else if (

r371766 - [Clang][CodeGen] support alias attribute w/ gnu_inline

2019-09-12 Thread Nick Desaulniers via cfe-commits 
Author: nickdesaulniers
Date: Thu Sep 12 12:53:35 2019
New Revision: 371766

URL: http://llvm.org/viewvc/llvm-project?rev=371766&view=rev
Log:
[Clang][CodeGen] support alias attribute w/ gnu_inline

Summary:
r369705 did not consider the addition of gnu_inline on function
declarations of alias attributed functions. This resulted in a reported
regression in the clang-9-rc4 release from the Zig developers building
glibc, which was observable as a failed assertion:

llvm-project/clang/lib/AST/Decl.cpp:3336: bool
clang::FunctionDecl::isInlineDefinitionExternallyVisible() const:
Assertion `(doesThisDeclarationHaveABody() || willHaveBody()) && "Must
be a function definition"' failed.

Alias function declarations do not have bodies, so allow us to proceed
if we have the alias function attribute but no body/definition, and add
a test case.  The emitted symbols and their linkage matches GCC for the
added test case.

Link: https://bugs.llvm.org/show_bug.cgi?id=43268

Reviewers: aaron.ballman, rsmith, erichkeane, andrewrk

Reviewed By: andrewrk

Subscribers: cfe-commits, andrewrk, hans, srhines

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/test/CodeGen/alias.c

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=371766&r1=371765&r2=371766&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Sep 12 12:53:35 2019
@@ -3348,7 +3348,8 @@ SourceRange FunctionDecl::getExceptionSp
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
+  assert((doesThisDeclarationHaveABody() || willHaveBody() ||
+  hasAttr()) &&
  "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();

Modified: cfe/trunk/test/CodeGen/alias.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/alias.c?rev=371766&r1=371765&r2=371766&view=diff
==
--- cfe/trunk/test/CodeGen/alias.c (original)
+++ cfe/trunk/test/CodeGen/alias.c Thu Sep 12 12:53:35 2019
@@ -99,3 +99,8 @@ static int test10_foo __attribute__((ali
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));


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

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

2019-09-12 Thread Nick Desaulniers via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371766: [Clang][CodeGen] support alias attribute w/ 
gnu_inline (authored by nickdesaulniers, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67455?vs=219790&id=219980#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67455

Files:
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/test/CodeGen/alias.c


Index: cfe/trunk/test/CodeGen/alias.c
===
--- cfe/trunk/test/CodeGen/alias.c
+++ cfe/trunk/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -3348,7 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
+  assert((doesThisDeclarationHaveABody() || willHaveBody() ||
+  hasAttr()) &&
  "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();


Index: cfe/trunk/test/CodeGen/alias.c
===
--- cfe/trunk/test/CodeGen/alias.c
+++ cfe/trunk/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -3348,7 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
+  assert((doesThisDeclarationHaveABody() || willHaveBody() ||
+  hasAttr()) &&
  "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371767 - Improve code generation for thread_local variables:

2019-09-12 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Thu Sep 12 13:00:24 2019
New Revision: 371767

URL: http://llvm.org/viewvc/llvm-project?rev=371767&view=rev
Log:
Improve code generation for thread_local variables:

Summary:
 * Don't bother using a thread wrapper when the variable is known to
   have constant initialization.
 * Emit the thread wrapper as discardable-if-unused in TUs that don't
   contain a definition of the thread_local variable.
 * Don't emit the thread wrapper at all if the thread_local variable
   is unused and discardable; it will be emitted by all TUs that need
   it.

Reviewers: rjmccall, jdoerfert

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
cfe/trunk/test/CodeGenCXX/windows-on-arm-itanium-thread-local.cpp
  - copied, changed from r371766, 
cfe/trunk/test/CodeGen/windows-on-arm-itanium-thread-local.c
Removed:
cfe/trunk/test/CodeGen/windows-on-arm-itanium-thread-local.c
Modified:
cfe/trunk/include/clang/Basic/Linkage.h
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp
cfe/trunk/test/CodeGenCXX/tls-init-funcs.cpp
cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp

Modified: cfe/trunk/include/clang/Basic/Linkage.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Linkage.h?rev=371767&r1=371766&r2=371767&view=diff
==
--- cfe/trunk/include/clang/Basic/Linkage.h (original)
+++ cfe/trunk/include/clang/Basic/Linkage.h Thu Sep 12 13:00:24 2019
@@ -82,6 +82,12 @@ inline bool isDiscardableGVALinkage(GVAL
   return L <= GVA_DiscardableODR;
 }
 
+/// Do we know that this will be the only definition of this symbol (excluding
+/// inlining-only definitions)?
+inline bool isUniqueGVALinkage(GVALinkage L) {
+  return L == GVA_Internal || L == GVA_StrongExternal;
+}
+
 inline bool isExternallyVisible(Linkage L) {
   return L >= VisibleNoLinkage;
 }

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=371767&r1=371766&r2=371767&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Thu Sep 12 13:00:24 2019
@@ -577,7 +577,7 @@ public:
 
   // Determine if references to thread_local global variables can be made
   // directly or require access through a thread wrapper function.
-  virtual bool usesThreadWrapperFunction() const = 0;
+  virtual bool usesThreadWrapperFunction(const VarDecl *VD) const = 0;
 
   /// Emit a reference to a non-local thread_local variable (including
   /// triggering the initialization of all thread_local variables in its

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=371767&r1=371766&r2=371767&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu Sep 12 13:00:24 2019
@@ -2361,7 +2361,7 @@ static LValue EmitGlobalVarDeclLValue(Co
 
   // If it's thread_local, emit a call to its wrapper function instead.
   if (VD->getTLSKind() == VarDecl::TLS_Dynamic &&
-  CGF.CGM.getCXXABI().usesThreadWrapperFunction())
+  CGF.CGM.getCXXABI().usesThreadWrapperFunction(VD))
 return CGF.CGM.getCXXABI().EmitThreadLocalVarDeclLValue(CGF, VD, T);
   // Check if the variable is marked as declare target with link clause in
   // device codegen.

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=371767&r1=371766&r2=371767&view=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu Sep 12 13:00:24 2019
@@ -43,6 +43,10 @@ class ItaniumCXXABI : public CodeGen::CG
   /// VTables - All the vtables which have been defined.
   llvm::DenseMap VTables;
 
+  /// All the thread wrapper functions that have been used.
+  llvm::SmallVector, 8>
+  ThreadWrappers;
+
 protected:
   bool UseARMMethodPtrABI;
   bool UseARMGuardVarABI;
@@ -322,7 +326,42 @@ public:
   ArrayRef CXXThreadLocalInits,
   ArrayRef CXXThreadLocalInitVars) override;
 
-  bool usesThreadWrapperFunction() const override { return true; }
+  /// Determine whether we will definitely emit this variable with a constant
+  /// initializer, either because the language semantics demand it or because
+  /// we know that the initializer is a constant.
+  bool isEmittedWithConstantInitializer(const VarDecl *VD) const {
+VD = VD->getMostRecentDecl();
+if (VD-

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

2019-09-12 Thread Borsik Gábor via Phabricator via cfe-commits 
boga95 updated this revision to Diff 219981.
boga95 marked 4 inline comments as done.

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

https://reviews.llvm.org/D59516

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/test/Analysis/Inputs/taint-generic-config.yaml
  clang/test/Analysis/taint-generic.c

Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -346,6 +346,12 @@
 void myScanf(const char*, ...);
 int myPropagator(int, int*);
 int mySnprintf(char*, size_t, const char*, ...);
+void myFilter1(int*);
+void myFilter2(int* x) {
+  if (*x < 0)
+*x = 0;
+}
+void myFilter3(const int*);
 void mySink(int, int, int);
 
 void testConfigurationSources1() {
@@ -372,6 +378,24 @@
   Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
 }
 
+void testConfigurationFilter1() {
+  int x = mySource1();
+  myFilter1(&x);
+  Buffer[x] = 1; // no-warning
+}
+
+void testConfigurationFilter2() {
+  int x = mySource1();
+  myFilter2(&x);
+  Buffer[x] = 1; // no-warning
+}
+
+void testConfigurationFilter3() {
+  int x = mySource1();
+  myFilter3(&x);
+  Buffer[x] = 1; // no-warning
+}
+
 void testConfigurationSinks() {
   int x = mySource1();
   mySink(x, 1, 2);
Index: clang/test/Analysis/Inputs/taint-generic-config.yaml
===
--- clang/test/Analysis/Inputs/taint-generic-config.yaml
+++ clang/test/Analysis/Inputs/taint-generic-config.yaml
@@ -36,8 +36,12 @@
 # A list of filter functions
 Filters:
   # int x; // x is tainted
-  # myFilter(&x); // x is not tainted anymore
-  - Name: myFilter
+  # myFilter1(&x); // x is not tainted anymore
+  - Name: myFilter1
+Args: [0]
+  - Name: myFilter2
+Args: [0]
+  - Name: myFilter3
 Args: [0]
 
 # A list of sink functions
Index: clang/lib/StaticAnalyzer/Checkers/Taint.h
===
--- clang/lib/StaticAnalyzer/Checkers/Taint.h
+++ clang/lib/StaticAnalyzer/Checkers/Taint.h
@@ -24,37 +24,35 @@
 /// taint.
 using TaintTagType = unsigned;
 
-static constexpr TaintTagType TaintTagGeneric = 0;
+static constexpr TaintTagType TaintTagNotTainted = 0;
+static constexpr TaintTagType TaintTagGeneric = 1;
 
 /// Create a new state in which the value of the statement is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, const Stmt *S, const LocationContext *LCtx,
- TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, const Stmt *S,
+const LocationContext *LCtx,
+TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the value is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, SVal V,
- TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SVal V,
+TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the symbol is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, SymbolRef Sym,
- TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SymbolRef Sym,
+TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the pointer represented by the region
 /// is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, const MemRegion *R,
- TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State,
+const MemRegion *R,
+TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in a which a sub-region of a given symbol is tainted.
 /// This might be necessary when referring to regions that can not have an
 /// individual symbol, e.g. if they are represented by the default binding of
 /// a LazyCompoundVal.
-LLVM_NODISCARD ProgramStateRef
-addPartialTaint(ProgramStateRef State,
-SymbolRef ParentSym, const SubRegion *SubRegion,
-TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addPartialTaint(
+ProgramStateRef State, SymbolRef ParentSym, const SubRegion *SubRegion,
+TaintTagType Kind = TaintTagGeneric);
 
 /// Check if the statement has a tainted value in the given state.
 bool isTainted(ProgramStateRef State, const Stmt *S,
@@ -99,4 +97,3 @@
 } // namespace clang
 
 #endif
-
Index: clang/lib/StaticAnalyzer/Checkers/Taint.cpp
===
--- clang/lib/S

[PATCH] D67429: Improve code generation for thread_local variables:

2019-09-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371767: Improve code generation for thread_local variables: 
(authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67429?vs=219652&id=219983#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67429

Files:
  cfe/trunk/include/clang/Basic/Linkage.h
  cfe/trunk/lib/CodeGen/CGCXXABI.h
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
  cfe/trunk/test/CodeGen/windows-on-arm-itanium-thread-local.c
  cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp
  cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
  cfe/trunk/test/CodeGenCXX/tls-init-funcs.cpp
  cfe/trunk/test/CodeGenCXX/windows-on-arm-itanium-thread-local.cpp
  cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp

Index: cfe/trunk/include/clang/Basic/Linkage.h
===
--- cfe/trunk/include/clang/Basic/Linkage.h
+++ cfe/trunk/include/clang/Basic/Linkage.h
@@ -82,6 +82,12 @@
   return L <= GVA_DiscardableODR;
 }
 
+/// Do we know that this will be the only definition of this symbol (excluding
+/// inlining-only definitions)?
+inline bool isUniqueGVALinkage(GVALinkage L) {
+  return L == GVA_Internal || L == GVA_StrongExternal;
+}
+
 inline bool isExternallyVisible(Linkage L) {
   return L >= VisibleNoLinkage;
 }
Index: cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
===
--- cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
+++ cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
@@ -101,8 +101,7 @@
   // LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]](
   // LAMBDA: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 0, {{.+}}* [[OMP_REGION:@.+]] to {{.+}})
 
-  // TLS-LAMBDA: [[G_CPY_VAL:%.+]] = call{{( cxx_fast_tlscc)?}} i{{[0-9]+}}* [[G_CTOR:@.+]]()
-  // TLS-LAMBDA: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* [[G_CPY_VAL]])
+  // TLS-LAMBDA: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* @g)
 
 #pragma omp parallel copyin(g)
   {
@@ -120,14 +119,12 @@
 // LAMBDA: [[DONE]]
 
 // TLS-LAMBDA-DAG: [[G_CAPTURE_SRC:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** %
-// TLS-LAMBDA-DAG: [[G_CAPTURE_DST:%.+]] = call{{( cxx_fast_tlscc)?}} i{{[0-9]+}}* [[G_CTOR]]()
 // TLS-LAMBDA-DAG: [[G_CAPTURE_SRCC:%.+]] = ptrtoint i{{[0-9]+}}* [[G_CAPTURE_SRC]] to i{{[0-9]+}}
-// TLS-LAMBDA-DAG: [[G_CAPTURE_DSTC:%.+]] = ptrtoint i{{[0-9]+}}* [[G_CAPTURE_DST]] to i{{[0-9]+}}
-// TLS-LAMBDA: icmp ne i{{[0-9]+}} {{%.+}}, {{%.+}}
+// TLS-LAMBDA: icmp ne i{{[0-9]+}} {{%.+}}, ptrtoint (i{{[0-9]+}}* @g to i{{[0-9]+}})
 // TLS-LAMBDA: br i1 %{{.+}}, label %[[NOT_MASTER:.+]], label %[[DONE:.+]]
 // TLS-LAMBDA: [[NOT_MASTER]]
 // TLS-LAMBDA: load i{{[0-9]+}}, i{{[0-9]+}}* [[G_CAPTURE_SRC]],
-// TLS-LAMBDA: store volatile i{{[0-9]+}} %{{.+}}, i{{[0-9]+}}* [[G_CAPTURE_DST]], align 128
+// TLS-LAMBDA: store volatile i{{[0-9]+}} %{{.+}}, i{{[0-9]+}}* @g, align 128
 // TLS-LAMBDA: [[DONE]]
 
 // LAMBDA: call {{.*}}void @__kmpc_barrier(
@@ -136,18 +133,13 @@
 // LAMBDA: call{{.*}} void [[INNER_LAMBDA:@.+]](%{{.+}}*
 // TLS-LAMBDA: call{{.*}} void [[INNER_LAMBDA:@.+]](%{{.+}}*
 
-// TLS-LAMBDA: define {{.*}}i{{[0-9]+}}* [[G_CTOR]]()
-// TLS-LAMBDA: ret i{{[0-9]+}}* [[G]]
-// TLS-LAMBDA: }
-
 [&]() {
   // LAMBDA: define {{.+}} void [[INNER_LAMBDA]](%{{.+}}* [[ARG_PTR:%.+]])
   // LAMBDA: store %{{.+}}* [[ARG_PTR]], %{{.+}}** [[ARG_PTR_REF:%.+]],
   g = 2;
   // LAMBDA: [[ARG_PTR:%.+]] = load %{{.+}}*, %{{.+}}** [[ARG_PTR_REF]]
 
-  // TLS-LAMBDA: [[G_CAPTURE_DST:%.+]] = call{{( cxx_fast_tlscc)?}} i{{[0-9]+}}* [[G_CTOR]]()
-  // TLS-LAMBDA: store volatile i{{[0-9]+}} 2, i{{[0-9]+}}* [[G_CAPTURE_DST]], align 128
+  // TLS-LAMBDA: store volatile i{{[0-9]+}} 2, i{{[0-9]+}}* @g, align 128
 }();
   }
   }();
@@ -164,8 +156,7 @@
   // BLOCKS: define{{.*}} internal{{.*}} void {{.+}}(i8*
   // BLOCKS: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 0, {{.+}}* [[OMP_REGION:@.+]] to {{.+}})
 
-  // TLS-BLOCKS: [[G_CPY_VAL:%.+]] = call{{( cxx_fast_tlscc)?}} i{{[0-9]+}}* [[G_CTOR:@.+]]()
-  // TLS-BLOCKS: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* [[G_CPY_VAL]])
+  // TLS-BLOCKS: call {{.*}}void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* @g)
 
 #pragma omp parallel copyin(g)
   {
@@ -183,14 +174,12 @@
 // BLOCKS: [[DONE]]
 
 // TLS-BLOCKS-DAG: [[G_CAPTURE_SRC:%.+]] = load i{{[0-9]+}}*, i{{

[PATCH] D67135: [clang-tidy] performance-inefficient-vector-operation: Support proto repeated field

2019-09-12 Thread Cong Liu via Phabricator via cfe-commits 
congliu updated this revision to Diff 219988.
congliu marked an inline comment as done.
congliu added a comment.

Addressed Haojian's comment.


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

https://reviews.llvm.org/D67135

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp

Index: clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
===
--- clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm
+// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
+// RUN: -format-style=llvm \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
 
 namespace std {
 
@@ -62,13 +65,35 @@
 
 int Op(int);
 
+namespace proto2 {
+class MessageLite {};
+class Message : public MessageLite {};
+} // namespace proto2
+
+class FooProto : public proto2::Message {
+ public:
+  int *add_x();  // repeated int x;
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+  int add_z() const; // optional int add_z;
+};
+
+class BarProto : public proto2::Message {
+ public:
+  int *add_x();
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+};
+
 void f(std::vector& t) {
   {
 std::vector v0;
 // CHECK-FIXES: v0.reserve(10);
 for (int i = 0; i < 10; ++i)
   v0.push_back(i);
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the vector capacity before the loop
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
   }
   {
 std::vector v1;
@@ -162,6 +187,15 @@
 }
   }
 
+  {
+FooProto foo;
+// CHECK-FIXES: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'add_x' is called inside a loop; consider pre-allocating the container capacity before the loop
+}
+  }
+
   //  Non-fixed Cases 
   {
 std::vector z0;
@@ -274,4 +308,54 @@
   z12.push_back(e);
 }
   }
+
+  {
+FooProto foo;
+foo.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+foo.add_x(-1);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+BarProto bar;
+bar.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x();
+  bar.add_x();
+}
+  }
+  {
+FooProto foo;
+foo.mutable_y();
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_z()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_z();
+}
+  }
 }
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
@@ -6,6 +6,10 @@
 Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``,
 ``emplace_back``) that may cause unnecessary memory reallocations.
 
+It can also find calls that add element to protobuf repeated field in a loop
+without calling Reserve() before the loop. Calling Reserve() first can avoid
+unnecessary memory reallocations.
+
 Currently, the check only detects following kinds of loops with a single
 statement body:
 
@@ -21,6 +25,13 @@
 // statement before the for statement.
   }
 
+  SomeProto p;
+  for (int i = 0; i < n; ++i) {
+p.add_xxx(n);
+// This will trigger the warning since the add_xxx may cause multiple memory
+// relloacations. This can be avoid by inserting a
+// 'p.mutable_xxx().Reserve(n)' statement before the for statement.
+  }
 
 * For-range loops like ``for (range-declaration : range_expression)``, the type
   of ``range_expression`` can be ``std::vector``, ``std::array``,
@@ -47,3 +5

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-09-12 Thread Jonathan Meier via Phabricator via cfe-commits 
jonathanmeier added a comment.

Thanks, this looks much better now. I think there's no need to track nesting of 
parenthesis, brackets and braces separately, so we can collapse `ParenLevel`, 
`BraceLevel` and `SquareBracketLevel` into a single `NestingLevel`, which 
simplifies all the conditions quite a bit.

Also consider adding a few more testcases, especially with nested templates and 
multiple template arguments (the reason for this patch, after all! ;-) ). You 
can easily test (different numbers of) multiple template arguments by just 
adding a few more template parameters with default arguments to e.g. 
`TwoArgTemplate`, `S` or `Q`.




Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:90
+  AngleBracketLevel == 0) {
+// If there is comma not nested then it is two or more declarations in 
this chain.
 return false;

change to `If there is a non-nested comma we have two or more declarations in 
this chain.`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

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

I'd like the test cases to actually demonstrate the correct use of the filters 
and the correct behavior of the Analyzer when the filters are annotated 
correctly, but it looks to me that they either demonstrate behavior when the 
annotation is //not// used correctly, or we disagree about how the taint should 
work in the first place. Testing the behavior when the annotation is misplaced 
is fine (with enough comments and probably FIXMEs where applicable), but 
"positive" tests are more valuable because they are the actual common cases 
(hopefully).




Comment at: clang/test/Analysis/taint-generic.c:381-385
+void testConfigurationFilter1() {
+  int x = mySource1();
+  myFilter1(&x);
+  Buffer[x] = 1; // no-warning
+}

`myFilter1` will create a new conjured symbol within `x` when evaluated 
conservatively. In this case there clearly shouldn't be a warning on `Buffer[x] 
= 1`, because nobody marked the new symbol as tainted to begin with. Therefore 
i think that this test doesn't really test the new functionality.



Comment at: clang/test/Analysis/taint-generic.c:387-391
+void testConfigurationFilter2() {
+  int x = mySource1();
+  myFilter2(&x);
+  Buffer[x] = 1; // no-warning
+}

Assuming `myFilter2` is inlined, we have two execution paths here. On one of 
them `x` is reset to a concrete 0. Because concrete values cannot carry taint 
to begin with, this execution path doesn't test the new functionality. On the 
other path `x` indeed has the old tainted value but it's now constrained to 
`[0, INT_MAX]`. Because it's still not constrained to be less than `BUFSIZE`, 
i'd say this looks more like a false negative and the new functionality makes 
the analysis worse. So this test does test the new functionality, but it kinda 
makes the new functionality look bad rather than good.



Comment at: clang/test/Analysis/taint-generic.c:393-397
+void testConfigurationFilter3() {
+  int x = mySource1();
+  myFilter3(&x);
+  Buffer[x] = 1; // no-warning
+}

In this example `myFilter3` promises not to alter the value of `x` due to 
`const`-qualifier on the pointee type of the parameter. Additionally, the 
function has no chance of preventing the program from reaching the buffer 
overflow line, other than crashing the program (given that it's C and there are 
no exceptions). Therefore i'd say this is also a false negative.

A better motivational example that doesn't immediately look like a false 
negative may look like this:
```lang=c
void testConfigurationFilter3() {
  int x = mySource1();
  if (isOutOfRange(x)) // the filter function
return;
  Buffer[x] = 1; // no-warning
}
```
In this case the function looks at the value of `x` (there's really no need to 
pass it by pointer) and notifies the caller through its return value that it is 
unsafe to access the buffer with such index. This is probably the only 
situation where a "filter" annotation is actually worth it.


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

https://reviews.llvm.org/D59516



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

[PATCH] D67516: [clang] Enable GNU Toolchain to find musl GCC installations

2019-09-12 Thread Elliot Saba via Phabricator via cfe-commits 
staticfloat created this revision.
staticfloat added reviewers: rsmith, fedor.sergeev.
staticfloat added a project: clang.
Herald added a subscriber: cfe-commits.

The GNU Toolchain GCCDetector misses GCC installations with the triplet 
`x86_64-linux-musl`, looking only instead for `x86_64-unknown-linux-musl` 
because it does not have the built-in shortcut that it does for glibc-based 
systems.  This patch, taken from the void-linux patch set at 
https://github.com/void-linux/void-packages/blob/ced162673bf1e824529eb04d8c33a4e30dcfb68e/srcpkgs/llvm8/files/patches/cfe/cfe-004-add-musl-triples.patch
 improves clang's ability to find these shorter triplets on `musl`.  This is 
necessary also for environments such as the Julia project's BinaryBuilder, 
which comprises a large collection of cross-compilers, each installed within a 
specific multiarch triplet.


Repository:
  rC Clang

https://reviews.llvm.org/D67516

Files:
  b/lib/Driver/ToolChains/Gnu.cpp


Index: b/lib/Driver/ToolChains/Gnu.cpp
===
--- b/lib/Driver/ToolChains/Gnu.cpp
+++ b/lib/Driver/ToolChains/Gnu.cpp
@@ -1882,7 +1882,8 @@
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
- "armv7hl-suse-linux-gnueabi"};
+ "armv7hl-suse-linux-gnueabi",
+ "armv7l-linux-gnueabihf"};
   static const char *const ARMebLibDirs[] = {"/lib"};
   static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
  "armeb-linux-androideabi"};
@@ -2014,6 +2015,78 @@
 return;
   }
 
+  if (TargetTriple.isMusl()) {
+static const char *const AArch64MuslTriples[] = {"aarch64-linux-musl"};
+static const char *const ARMHFMuslTriples[] = {
+"arm-linux-musleabihf", "armv7l-linux-musleabihf"
+};
+static const char *const ARMMuslTriples[] = {"arm-linux-musleabi"};
+static const char *const X86_64MuslTriples[] = {"x86_64-linux-musl"};
+static const char *const X86MuslTriples[] = {"i686-linux-musl"};
+static const char *const MIPSMuslTriples[] = {
+"mips-linux-musl", "mipsel-linux-musl",
+"mipsel-linux-muslhf", "mips-linux-muslhf"
+};
+static const char *const PPCMuslTriples[] = {"powerpc-linux-musl"};
+static const char *const PPC64MuslTriples[] = {"powerpc64-linux-musl"};
+static const char *const PPC64LEMuslTriples[] = {"powerpc64le-linux-musl"};
+
+switch (TargetTriple.getArch()) {
+case llvm::Triple::aarch64:
+  LibDirs.append(begin(AArch64LibDirs), end(AArch64LibDirs));
+  TripleAliases.append(begin(AArch64MuslTriples), end(AArch64MuslTriples));
+  BiarchLibDirs.append(begin(AArch64LibDirs), end(AArch64LibDirs));
+  BiarchTripleAliases.append(begin(AArch64MuslTriples), 
end(AArch64MuslTriples));
+  break;
+case llvm::Triple::arm:
+  LibDirs.append(begin(ARMLibDirs), end(ARMLibDirs));
+  if (TargetTriple.getEnvironment() == llvm::Triple::MuslEABIHF) {
+TripleAliases.append(begin(ARMHFMuslTriples), end(ARMHFMuslTriples));
+  } else {
+TripleAliases.append(begin(ARMMuslTriples), end(ARMMuslTriples));
+  }
+  break;
+case llvm::Triple::x86_64:
+  LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
+  TripleAliases.append(begin(X86_64MuslTriples), end(X86_64MuslTriples));
+  BiarchLibDirs.append(begin(X86LibDirs), end(X86LibDirs));
+  BiarchTripleAliases.append(begin(X86MuslTriples), end(X86MuslTriples));
+  break;
+case llvm::Triple::x86:
+  LibDirs.append(begin(X86LibDirs), end(X86LibDirs));
+  TripleAliases.append(begin(X86MuslTriples), end(X86MuslTriples));
+  BiarchLibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
+  BiarchTripleAliases.append(begin(X86_64MuslTriples), 
end(X86_64MuslTriples));
+  break;
+case llvm::Triple::mips:
+  LibDirs.append(begin(MIPSLibDirs), end(MIPSLibDirs));
+  TripleAliases.append(begin(MIPSMuslTriples), end(MIPSMuslTriples));
+  break;
+case llvm::Triple::ppc:
+  LibDirs.append(begin(PPCLibDirs), end(PPCLibDirs));
+  TripleAliases.append(begin(PPCMuslTriples), end(PPCMuslTriples));
+  BiarchLibDirs.append(begin(PPC64LibDirs), end(PPC64LibDirs));
+  BiarchTripleAliases.append(begin(PPC64MuslTriples), 
end(PPC64MuslTriples));
+  break;
+case llvm::Triple::ppc64:
+  LibDirs.append(begin(PPC64LibDirs), end(PPC64LibDirs));
+  TripleAliases.append(begin(PPC64MuslTriples), end(PPC64MuslTriples));
+  BiarchLibDirs.append(begin(PPCLibDirs), end(PPCLibDirs));
+  BiarchTripleAliases.append(begin(PPCMuslTriples), end(PPCMuslTriples));
+  break;
+case llvm::Triple::ppc64le:
+  LibDirs.

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-09-12 Thread Melanie Blower via Phabricator via cfe-commits 
mibintc added a comment.

In D62731#1663748 , @rjmccall wrote:

> Hmm, you know, there are enough different FP options that I think we should 
> probably split them all out into their own section in the manual instead of 
> just listing them under "code generation".  That will also give us an obvious 
> place to describe the basic model, i.e. all the stuff about it mostly coming 
> down to different strictness and exception models.  Could you prepare a patch 
> that *just* does that reorganization without adding any new features, and 
> then we can add the new options on top of that?


I uploaded a patch to move floating point options to a new documentation 
section here, https://reviews.llvm.org/D67517


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

2019-09-12 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added inline comments.



Comment at: lib/AST/Decl.cpp:3283
 
   if (Context.getLangOpts().CPlusPlus)
 return false;

I would have thought the existing case here would handle your change.  If it 
doesn't, why not?  Should your change also remove this (essentially moving it 
earlier)?



Comment at: lib/AST/Decl.cpp:3381
   // The rest of this function is C-only.
   assert(!Context.getLangOpts().CPlusPlus &&
  "should not use C inline rules in C++");

Ditto.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67414



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

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

2019-09-12 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.



Comment at: lib/AST/Decl.cpp:3283
 
   if (Context.getLangOpts().CPlusPlus)
 return false;

nickdesaulniers wrote:
> I would have thought the existing case here would handle your change.  If it 
> doesn't, why not?  Should your change also remove this (essentially moving it 
> earlier)?
With the `gnu_inline` attribute on a function, the block above can end up 
returning `FoundBody = true`.

So yes, I guess one would get the same effect by just moving the existing if 
statement up past the gnu inline block.



Comment at: lib/AST/Decl.cpp:3381
   // The rest of this function is C-only.
   assert(!Context.getLangOpts().CPlusPlus &&
  "should not use C inline rules in C++");

nickdesaulniers wrote:
> Ditto.
Hmm, if moved above, I guess we should convert the assert to a plain `if 
(CPlusPlus) return false;` instead?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67414



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

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

2019-09-12 Thread Kito Cheng via Phabricator via cfe-commits 
kito-cheng added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:387
+
+void riscv::addGoldOptions(const ToolChain &ToolChain,
+   const llvm::opt::ArgList &Args,

MaskRay wrote:
> gold doesn't support RISC-V, does it?
Gold doesn't support RISC-V , but ld.bfd supported same plugin API, so this 
made clang can run LTO with ld.bfd.


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

https://reviews.llvm.org/D67409



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

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

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

Seems very surprising to me that the `gnu_inline` attribute has different 
behavior in their C and C++ frontends. That said, it looks like it's a behavior 
that they document; their documentation says "In C++, this attribute does not 
depend on extern in any way, but it still requires the inline keyword to enable 
its special behavior." and that matches their observed behavior. And they 
behave this way even for functions in `extern "C"` blocks. The question for us, 
then, is do we want to be compatible with C source code built as C++ (that is, 
the status quo) and C++ source code using this attribute and targeting older 
versions of Clang, or with g++?

Is Clang's current behavior actually causing problems for some real use case? 
Changing this behavior is not without risk, and the old behavior is certainly 
defensible.

If we want to match GCC's behavior for `gnu_inline` in C++ mode, we should 
think about whether we also want to permit things like:

  __attribute__((__gnu_inline__)) extern inline int externinlinefunc() { return 
0; }
  int externinlinefunc() { return 1; }

(with or without the `extern`), which g++ allows and clang rejects.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67414



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

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

In D67420#1668099 , @Szelethus wrote:

> ...and the second is similar in nature, but in the actual code -- it doesn't 
> doesn't feel natural to me that `AnalyzerOptions` is required to construct 
> this, while at the same time we're trying to make diagnostics construction 
> independent of the analyzer.


It's not that `AnalyzerOptions` is //necessary// to construct this, it's more 
like //sufficient//. `PathDiagnosticConsumerOptions` is a plain de-encapsulated 
aggregate and anybody can aggregate-initialize or mutate it. But in the realm 
of the Analyzer it has "accidentally" turned out that `AnalyzerOptions` has 
just the right amount of information to fill it in.


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

https://reviews.llvm.org/D67420



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

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

2019-09-12 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

In D67414#1668443 , @rsmith wrote:

> Seems very surprising to me that the `gnu_inline` attribute has different 
> behavior in their C and C++ frontends. That said, it looks like it's a 
> behavior that they document; their documentation says "In C++, this attribute 
> does not depend on extern in any way, but it still requires the inline 
> keyword to enable its special behavior." and that matches their observed 
> behavior. And they behave this way even for functions in `extern "C"` blocks. 
> The question for us, then, is do we want to be compatible with C source code 
> built as C++ (that is, the status quo) and C++ source code using this 
> attribute and targeting older versions of Clang, or with g++?


I think it's a rather uncommon combination (I think most use of gnu_inline is 
together with extern), so I wouldn't think that there's a lot of users relying 
on the current behaviour (in clang-only codebases), but I have no data to back 
it up with.

> Is Clang's current behavior actually causing problems for some real use case? 
> Changing this behavior is not without risk, and the old behavior is certainly 
> defensible.

It did come up in mingw-w64 recently; new header additions (that were developed 
with gcc) broke when built with clang (due to multiple definitions of a 
symbol). It's been rectified for now by avoiding this combination (for 
compatibility with existing clang versions) though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67414



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

[clang-tools-extra] r371773 - [ClangTidy] Adjust the name getCheckName to getCheckerName due to API change.

2019-09-12 Thread Tim Shen via cfe-commits 
Author: timshen
Date: Thu Sep 12 14:18:44 2019
New Revision: 371773

URL: http://llvm.org/viewvc/llvm-project?rev=371773&view=rev
Log:
[ClangTidy] Adjust the name getCheckName to getCheckerName due to API change.

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=371773&r1=371772&r2=371773&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu Sep 12 14:18:44 2019
@@ -72,7 +72,7 @@ public:
 FilesMade *filesMade) override {
 for (const ento::PathDiagnostic *PD : Diags) {
   SmallString<64> CheckName(AnalyzerCheckNamePrefix);
-  CheckName += PD->getCheckName();
+  CheckName += PD->getCheckerName();
   Context.diag(CheckName, PD->getLocation().asLocation(),
PD->getShortDescription())
   << PD->path.back()->getRanges();


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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

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

In D67420#1668474 , @NoQ wrote:

> It's not that `AnalyzerOptions` is //necessary// to construct this, it's more 
> like //sufficient//.


*sudden enlightenment*

Ah, okay, right, I'm a dummie, I think I got what's happening here! :) In that 
case, the idea sounds great. I'll take a second look on the code later (I have 
a feeling that we should delete the default ctor of 
`PathDiagnosticConsumerOptions`, etc), but I'm just a tad too tired atm :)


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

https://reviews.llvm.org/D67420



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

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

2019-09-12 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a subscriber: timshen.
NoQ added a comment.

rL371773  - thanks @timshen!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67140



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

  1   2   >