[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

2019-08-01 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment.

Hi @lenary, sure I can rebase this.

However, I think it may be better to do the `lp64d` change in another phab so 
we can keep this one NFC.


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

https://reviews.llvm.org/D48357



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


[PATCH] D65564: Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Rui Ueyama via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367649: Improve raw_ostream so that you can 
write colors using operator (authored by ruiu, committed by 
).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D65564?vs=212959=212968#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65564

Files:
  cfe/trunk/include/clang/AST/ASTDumperUtils.h
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/TextDiagnostic.cpp
  cfe/trunk/tools/diagtool/TreeView.cpp
  lld/trunk/COFF/Driver.cpp
  lld/trunk/COFF/DriverUtils.cpp
  lld/trunk/Common/ErrorHandler.cpp
  lld/trunk/ELF/Driver.cpp
  lld/trunk/ELF/DriverUtils.cpp
  lld/trunk/include/lld/Common/ErrorHandler.h
  lld/trunk/lib/Driver/DarwinLdDriver.cpp
  lld/trunk/test/COFF/color-diagnostics.test
  lld/trunk/test/ELF/color-diagnostics.test
  lld/trunk/wasm/Driver.cpp
  llvm/trunk/include/llvm/Support/FormattedStream.h
  llvm/trunk/include/llvm/Support/WithColor.h
  llvm/trunk/include/llvm/Support/raw_ostream.h
  llvm/trunk/lib/Support/WithColor.cpp
  llvm/trunk/lib/Support/raw_ostream.cpp
  llvm/trunk/tools/llvm-cov/CoverageReport.cpp
  llvm/trunk/tools/llvm-cov/CoverageViewOptions.h
  llvm/trunk/tools/llvm-cov/RenderingSupport.h
  llvm/trunk/tools/llvm-cov/SourceCoverageViewText.cpp
  llvm/trunk/tools/llvm-cov/llvm-cov.cpp
  llvm/trunk/tools/llvm-mca/Views/TimelineView.cpp
  llvm/trunk/utils/FileCheck/FileCheck.cpp

Index: lld/trunk/include/lld/Common/ErrorHandler.h
===
--- lld/trunk/include/lld/Common/ErrorHandler.h
+++ lld/trunk/include/lld/Common/ErrorHandler.h
@@ -87,7 +87,6 @@
   StringRef errorLimitExceededMsg = "too many errors emitted, stopping now";
   StringRef logName = "lld";
   llvm::raw_ostream *errorOS = ::errs();
-  bool colorDiagnostics = llvm::errs().has_colors();
   bool exitEarly = true;
   bool fatalWarnings = false;
   bool verbose = false;
@@ -102,9 +101,9 @@
   std::unique_ptr outputBuffer;
 
 private:
-  void printHeader(StringRef s, raw_ostream::Colors c, const Twine );
-  void printErrorMsg(const Twine );
-  void printError(const Twine );
+  using Color = raw_ostream::Color;
+
+  std::string getLocation(const Twine );
 };
 
 /// Returns the default error handler.
Index: lld/trunk/test/ELF/color-diagnostics.test
===
--- lld/trunk/test/ELF/color-diagnostics.test
+++ lld/trunk/test/ELF/color-diagnostics.test
@@ -6,8 +6,8 @@
 # RUN: not ld.lld -xyz -color-diagnostics=always /nosuchfile 2>&1 \
 # RUN:   | FileCheck -check-prefix=COLOR %s
 
-# COLOR: {{ld.lld: .\[0;1;31merror: .\[0munknown argument '-xyz'}}
-# COLOR: {{ld.lld: .\[0;1;31merror: .\[0mcannot open /nosuchfile}}
+# COLOR: {{ld.lld: .\[0;31merror: .\[0munknown argument '-xyz'}}
+# COLOR: {{ld.lld: .\[0;31merror: .\[0mcannot open /nosuchfile}}
 
 # RUN: not ld.lld -color-diagnostics=foobar 2>&1 | FileCheck -check-prefix=ERR %s
 # ERR: unknown option: --color-diagnostics=foobar
Index: lld/trunk/test/COFF/color-diagnostics.test
===
--- lld/trunk/test/COFF/color-diagnostics.test
+++ lld/trunk/test/COFF/color-diagnostics.test
@@ -6,8 +6,8 @@
 # RUN: not lld-link -xyz --color-diagnostics=always /nosuchfile 2>&1 \
 # RUN:   | FileCheck -check-prefix=COLOR %s
 
-# COLOR: {{lld-link: .\[0;1;35mwarning: .\[0mignoring unknown argument '-xyz'}}
-# COLOR: {{lld-link: .\[0;1;31merror: .\[0mcould not open '/nosuchfile'}}
+# COLOR: {{lld-link: .\[0;35mwarning: .\[0mignoring unknown argument '-xyz'}}
+# COLOR: {{lld-link: .\[0;31merror: .\[0mcould not open '/nosuchfile'}}
 
 # RUN: not lld-link /nosuchfile 2>&1 | FileCheck -check-prefix=NOCOLOR %s
 # RUN: not lld-link -color-diagnostics=never /nosuchfile 2>&1 \
Index: lld/trunk/COFF/Driver.cpp
===
--- lld/trunk/COFF/Driver.cpp
+++ lld/trunk/COFF/Driver.cpp
@@ -62,7 +62,6 @@
 bool link(ArrayRef args, bool canExitEarly, raw_ostream ) {
   errorHandler().logName = args::getFilenameWithoutExe(args[0]);
   errorHandler().errorOS = 
-  errorHandler().colorDiagnostics = diag.has_colors();
   errorHandler().errorLimitExceededMsg =
   "too many errors emitted, stopping now"
   " (use /errorlimit:0 to see all errors)";
Index: lld/trunk/COFF/DriverUtils.cpp
===
--- lld/trunk/COFF/DriverUtils.cpp
+++ lld/trunk/COFF/DriverUtils.cpp
@@ -756,16 +756,17 @@
   OPT_no_color_diagnostics);
   if (!arg)
 return;
+
   if (arg->getOption().getID() == OPT_color_diagnostics) {
-errorHandler().colorDiagnostics = true;
+errorHandler().errorOS->enable_colors();
   } else if (arg->getOption().getID() == 

r367649 - Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Rui Ueyama via cfe-commits
Author: ruiu
Date: Thu Aug  1 21:48:30 2019
New Revision: 367649

URL: http://llvm.org/viewvc/llvm-project?rev=367649=rev
Log:
Improve raw_ostream so that you can "write" colors using operator<<

1. raw_ostream supports ANSI colors so that you can write messages to
the termina with colors. Previously, in order to change and reset
color, you had to call `changeColor` and `resetColor` functions,
respectively.

So, if you print out "error: " in red, for example, you had to do
something like this:

  OS.changeColor(raw_ostream::RED);
  OS << "error: ";
  OS.resetColor();

With this patch, you can write the same code as follows:

  OS << raw_ostream::RED << "error: " << raw_ostream::RESET;

2. Add a boolean flag to raw_ostream so that you can disable colored
output. If you disable colors, changeColor, operator<<(Color),
resetColor and other color-related functions have no effect.

Most LLVM tools automatically prints out messages using colors, and
you can disable it by passing a flag such as `--disable-colors`.
This new flag makes it easy to write code that works that way.

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

Modified:
cfe/trunk/include/clang/AST/ASTDumperUtils.h
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/TextDiagnostic.cpp
cfe/trunk/tools/diagtool/TreeView.cpp

Modified: cfe/trunk/include/clang/AST/ASTDumperUtils.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTDumperUtils.h?rev=367649=367648=367649=diff
==
--- cfe/trunk/include/clang/AST/ASTDumperUtils.h (original)
+++ cfe/trunk/include/clang/AST/ASTDumperUtils.h Thu Aug  1 21:48:30 2019
@@ -27,7 +27,7 @@ enum ASTDumpOutputFormat {
 // Do not use bold yellow for any text.  It is hard to read on white screens.
 
 struct TerminalColor {
-  llvm::raw_ostream::Colors Color;
+  llvm::raw_ostream::Color Color;
   bool Bold;
 };
 

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=367649=367648=367649=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Thu Aug  1 21:48:30 2019
@@ -5509,7 +5509,7 @@ static void print_block(raw_ostream ,
   if (print_edges) {
 // Print the predecessors of this block.
 if (!B.pred_empty()) {
-  const raw_ostream::Colors Color = raw_ostream::BLUE;
+  const raw_ostream::Color Color = raw_ostream::BLUE;
   if (ShowColors)
 OS.changeColor(Color);
   OS << "   Preds " ;
@@ -5546,7 +5546,7 @@ static void print_block(raw_ostream ,
 
 // Print the successors of this block.
 if (!B.succ_empty()) {
-  const raw_ostream::Colors Color = raw_ostream::MAGENTA;
+  const raw_ostream::Color Color = raw_ostream::MAGENTA;
   if (ShowColors)
 OS.changeColor(Color);
   OS << "   Succs ";

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=367649=367648=367649=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Aug  1 21:48:30 2019
@@ -1491,6 +1491,10 @@ bool clang::ParseDiagnosticArgs(Diagnost
OPT_fno_diagnostics_show_option, DefaultShowOpt);
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
+  if (Opts.ShowColors) {
+llvm::outs().enable_colors();
+llvm::errs().enable_colors();
+  }
 
   // Default behavior is to not to show note include stacks.
   Opts.ShowNoteIncludeStack = false;

Modified: cfe/trunk/lib/Frontend/TextDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnostic.cpp?rev=367649=367648=367649=diff
==
--- cfe/trunk/lib/Frontend/TextDiagnostic.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnostic.cpp Thu Aug  1 21:48:30 2019
@@ -23,23 +23,16 @@
 
 using namespace clang;
 
-static const enum raw_ostream::Colors noteColor =
-  raw_ostream::BLACK;
-static const enum raw_ostream::Colors remarkColor =
-  raw_ostream::BLUE;
-static const enum raw_ostream::Colors fixitColor =
-  raw_ostream::GREEN;
-static const enum raw_ostream::Colors caretColor =
-  raw_ostream::GREEN;
-static const enum raw_ostream::Colors warningColor =
-  raw_ostream::MAGENTA;
-static const enum raw_ostream::Colors templateColor =
-  raw_ostream::CYAN;
-static const enum raw_ostream::Colors errorColor = raw_ostream::RED;
-static const enum raw_ostream::Colors fatalColor = raw_ostream::RED;
+static const raw_ostream::Color noteColor = raw_ostream::BLACK;
+static const raw_ostream::Color remarkColor = raw_ostream::BLUE;
+static const 

[PATCH] D65631: [clang-format] Fix a bug that doesn't break braces before unions for Allman

2019-08-01 Thread Owen Pan via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367648: [clang-format] Fix a bug that doesnt break 
braces before unions for Allman (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65631?vs=212964=212966#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65631

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


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r367648 - [clang-format] Fix a bug that doesn't break braces before unions for Allman

2019-08-01 Thread Owen Pan via cfe-commits
Author: owenpan
Date: Thu Aug  1 21:30:42 2019
New Revision: 367648

URL: http://llvm.org/viewvc/llvm-project?rev=367648=rev
Log:
[clang-format] Fix a bug that doesn't break braces before unions for Allman
Differential Revision: https://reviews.llvm.org/D65631

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

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=367648=367647=367648=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Aug  1 21:30:42 2019
@@ -640,6 +640,7 @@ static FormatStyle expandPresets(const F
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=367648=367647=367648=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Aug  1 21:30:42 2019
@@ -11086,6 +11086,9 @@ TEST_F(FormatTest, AllmanBraceBreaking)
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 


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


[PATCH] D65631: [clang-format] Fix a bug that doesn't break braces before unions for Allman

2019-08-01 Thread Owen Pan via Phabricator via cfe-commits
owenpan created this revision.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR42155


Repository:
  rC Clang

https://reviews.llvm.org/D65631

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65192: [Sema] Make -Wbitwise-op-parentheses and -Wlogical-op-parentheses disabled-by-default

2019-08-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 212962.
MaskRay added a comment.

test/Parser/cxx2a-spaceship.cpp does not need a change


Repository:
  rC Clang

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

https://reviews.llvm.org/D65192

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/SemaCXX/parentheses.cpp


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5642,10 +5642,10 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5642,10 +5642,10 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65564: Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu marked an inline comment as done.
ruiu added inline comments.



Comment at: clang/tools/diagtool/TreeView.cpp:30
 
-  TreePrinter(llvm::raw_ostream )
-  : out(out), ShowColors(hasColors(out)), Internal(false) {}
-
-  void setColor(llvm::raw_ostream::Colors Color) {
-if (ShowColors)
-  out << llvm::sys::Process::OutputColor(Color, false, false);
-  }
-
-  void resetColor() {
-if (ShowColors)
-  out << llvm::sys::Process::ResetColor();
+  TreePrinter(llvm::raw_ostream ) : out(out), Internal(false) {
+if ( != ::errs() &&  != ::outs())

grimar wrote:
> nit: Seems `out` should be uppercase here.
> (I see it was like that before your changes, but seems you touching all the 
> places where it was used, so seems you can fix it).
Yes, this is inconsistent, but we probably shouldn't change this in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65564



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


[PATCH] D65564: Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu updated this revision to Diff 212959.
ruiu added a comment.

- rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65564

Files:
  clang/include/clang/AST/ASTDumperUtils.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/tools/diagtool/TreeView.cpp
  lld/COFF/Driver.cpp
  lld/COFF/DriverUtils.cpp
  lld/Common/ErrorHandler.cpp
  lld/ELF/Driver.cpp
  lld/ELF/DriverUtils.cpp
  lld/include/lld/Common/ErrorHandler.h
  lld/lib/Driver/DarwinLdDriver.cpp
  lld/test/COFF/color-diagnostics.test
  lld/test/ELF/color-diagnostics.test
  lld/wasm/Driver.cpp
  llvm/include/llvm/Support/FormattedStream.h
  llvm/include/llvm/Support/WithColor.h
  llvm/include/llvm/Support/raw_ostream.h
  llvm/lib/Support/WithColor.cpp
  llvm/lib/Support/raw_ostream.cpp
  llvm/tools/llvm-cov/CoverageReport.cpp
  llvm/tools/llvm-cov/CoverageViewOptions.h
  llvm/tools/llvm-cov/RenderingSupport.h
  llvm/tools/llvm-cov/SourceCoverageViewText.cpp
  llvm/tools/llvm-cov/llvm-cov.cpp
  llvm/tools/llvm-mca/Views/TimelineView.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -138,12 +138,11 @@
   /// The starting char (before tildes) for marking the line.
   char Lead;
   /// What color to use for this annotation.
-  raw_ostream::Colors Color;
+  raw_ostream::Color Color;
   /// A note to follow the marker, or empty string if none.
   std::string Note;
   MarkerStyle() {}
-  MarkerStyle(char Lead, raw_ostream::Colors Color,
-  const std::string  = "")
+  MarkerStyle(char Lead, raw_ostream::Color Color, const std::string  = "")
   : Lead(Lead), Color(Color), Note(Note) {}
 };
 
Index: llvm/tools/llvm-mca/Views/TimelineView.cpp
===
--- llvm/tools/llvm-mca/Views/TimelineView.cpp
+++ llvm/tools/llvm-mca/Views/TimelineView.cpp
@@ -103,8 +103,8 @@
 LastCycle = std::max(LastCycle, CurrentCycle);
 }
 
-static raw_ostream::Colors chooseColor(unsigned CumulativeCycles,
-   unsigned Executions, int BufferSize) {
+static raw_ostream::Color chooseColor(unsigned CumulativeCycles,
+  unsigned Executions, int BufferSize) {
   if (CumulativeCycles && BufferSize < 0)
 return raw_ostream::MAGENTA;
   unsigned Size = static_cast(BufferSize);
@@ -120,7 +120,7 @@
   if (!OS.has_colors())
 return;
 
-  raw_ostream::Colors Color = chooseColor(Cycles, Executions, BufferSize);
+  raw_ostream::Color Color = chooseColor(Cycles, Executions, BufferSize);
   if (Color == raw_ostream::SAVEDCOLOR) {
 OS.resetColor();
 return;
Index: llvm/tools/llvm-cov/llvm-cov.cpp
===
--- llvm/tools/llvm-cov/llvm-cov.cpp
+++ llvm/tools/llvm-cov/llvm-cov.cpp
@@ -83,13 +83,10 @@
 }
   }
 
-  if (argc > 1) {
-if (sys::Process::StandardErrHasColors())
-  errs().changeColor(raw_ostream::RED);
-errs() << "Unrecognized command: " << argv[1] << ".\n\n";
-if (sys::Process::StandardErrHasColors())
-  errs().resetColor();
-  }
+  if (argc > 1)
+errs() << raw_ostream::RED << "Unrecognized command: " << argv[1] << ".\n\n"
+   << raw_ostream::RESET;
+
   helpMain(argc, argv);
   return 1;
 }
Index: llvm/tools/llvm-cov/SourceCoverageViewText.cpp
===
--- llvm/tools/llvm-cov/SourceCoverageViewText.cpp
+++ llvm/tools/llvm-cov/SourceCoverageViewText.cpp
@@ -101,7 +101,7 @@
   auto *WrappedSegment = LCS.getWrappedSegment();
   CoverageSegmentArray Segments = LCS.getLineSegments();
 
-  Optional Highlight;
+  Optional Highlight;
   SmallVector, 2> HighlightedRanges;
 
   // The first segment overlaps from a previous line, so we treat it specially.
Index: llvm/tools/llvm-cov/RenderingSupport.h
===
--- llvm/tools/llvm-cov/RenderingSupport.h
+++ llvm/tools/llvm-cov/RenderingSupport.h
@@ -47,7 +47,7 @@
 /// Change the color of the output stream if the `IsColorUsed` flag
 /// is true. Returns an object that resets the color when destroyed.
 inline ColoredRawOstream colored_ostream(raw_ostream ,
- raw_ostream::Colors Color,
+ raw_ostream::Color Color,
  bool IsColorUsed = true,
  bool Bold = false, bool BG = false) {
   if (IsColorUsed)
Index: llvm/tools/llvm-cov/CoverageViewOptions.h
===
--- llvm/tools/llvm-cov/CoverageViewOptions.h
+++ llvm/tools/llvm-cov/CoverageViewOptions.h
@@ -46,7 +46,7 @@
 
   /// Change 

[PATCH] D65192: [Sema] Make -Wbitwise-op-parentheses and -Wlogical-op-parentheses disabled-by-default

2019-08-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 212958.
MaskRay retitled this revision from "[Sema] Disable some enabled-by-default 
-Wparentheses diagnostics" to "[Sema] Make -Wbitwise-op-parentheses and 
-Wlogical-op-parentheses disabled-by-default".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Retitle

disable just -Wbitwise-op-parentheses and -Wlogical-op-parentheses


Repository:
  rC Clang

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

https://reviews.llvm.org/D65192

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Parser/cxx2a-spaceship.cpp
  test/SemaCXX/parentheses.cpp


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5642,10 +5642,10 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "


Index: test/SemaCXX/parentheses.cpp
===
--- test/SemaCXX/parentheses.cpp
+++ test/SemaCXX/parentheses.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s
 
 // PR16930, PR16727:
 template
Index: test/Parser/cxx2a-spaceship.cpp
===
--- test/Parser/cxx2a-spaceship.cpp
+++ test/Parser/cxx2a-spaceship.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s
+// RUN: %clang_cc1 -std=c++2a -verify -Wparentheses %s
 
 template struct X {};
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5642,10 +5642,10 @@
   "remove constant to silence this warning">;
 
 def warn_bitwise_op_in_bitwise_op : Warning<
-  "'%0' within '%1'">, InGroup;
+  "'%0' within '%1'">, InGroup, DefaultIgnore;
 
 def warn_logical_and_in_logical_or : Warning<
-  "'&&' within '||'">, InGroup;
+  "'&&' within '||'">, InGroup, DefaultIgnore;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

2019-08-01 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 212957.
jdoerfert added a comment.

Improve based on two comments by @Hahnfeld


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64375

Files:
  clang/docs/OpenMPSupport.rst

Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -2,12 +2,12 @@
 
   
 .none { background-color: #FF }
-.partial { background-color: #99 }
+.part { background-color: #99 }
 .good { background-color: #CCFF99 }
   
 
 .. role:: none
-.. role:: partial
+.. role:: part
 .. role:: good
 
 .. contents::
@@ -17,7 +17,7 @@
 OpenMP Support
 ==
 
-Clang supports the following OpenMP 5.0 features
+Clang supports the following OpenMP 5.0 features (see also `OpenMP implementation details`_):
 
 * The `reduction`-based clauses in the `task` and `target`-based directives.
 
@@ -37,7 +37,7 @@
 Clang fully supports OpenMP 4.5. Clang supports offloading to X86_64, AArch64,
 PPC64[LE] and has `basic support for Cuda devices`_.
 
-* #pragma omp declare simd: :partial:`Partial`.  We support parsing/semantic
+* #pragma omp declare simd: :part:`Partial`.  We support parsing/semantic
   analysis + generation of special attributes for X86 target, but still
   missing the LLVM pass for vectorization.
 
@@ -129,3 +129,134 @@
   In some cases the local variables are actually allocated in the global memory,
   but the debug info may be not aware of it.
 
+
+.. _OpenMP implementation details:
+
+OpenMP 5.0 Implementation Details
+-
+
+The following table provides a quick overview over various OpenMP 5.0 features
+and their implementation status. Please contact *openmp-dev* at
+*lists.llvm.org* for more information or if you want to help with the
+implementation.
+
++--+--+--++
+|Category  | Feature  | Status   | Reviews|
++==+==+==++
+| loop extension   | support != in the canonical loop form| :good:`done` | D54441 |
++--+--+--++
+| loop extension   | #pragma omp loop (directive) | :none:`unclaimed`||
++--+--+--++
+| loop extension   | collapse imperfectly nested loop | :none:`unclaimed`||
++--+--+--++
+| loop extension   | collapse non-rectangular nested loop | :part:`worked on`||
++--+--+--++
+| loop extension   | C++ range-base for loop  | :none:`unclaimed`||
++--+--+--++
+| loop extension   | clause: nosimd for SIMD directives   | :none:`unclaimed`||
++--+--+--++
+| loop extension   | inclusive scan extension (matching C++17 PSTL)   | :none:`unclaimed`||
++--+--+--++
+| memory mangagement   | memory allocators| :good:`done` | r341687,r357929|

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

2019-08-01 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Can you reduce this patch to only the `&&` within `||` and `&` within `|` 
changes? I think we have reasonable consensus that that should not be enabled 
by default, so let's land that part now. If you want to continue discussing 
`-Wshift-op-parentheses` after that, we can.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65192



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


[PATCH] D65629: cfi-icall: Allow the jump table to be optionally made non-canonical.

2019-08-01 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision.
pcc added reviewers: hctim, tejohnson.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, 
mehdi_amini.
Herald added projects: clang, LLVM.

The default behavior of Clang's indirect function call checker will replace
the address of each CFI-checked function in the output file's symbol table
with the address of a jump table entry which will pass CFI checks. We refer
to this as making the jump table `canonical`. This property allows code that
was not compiled with ``-fsanitize=cfi-icall`` to take a CFI-valid address
of a function, but it comes with a couple of caveats that are especially
relevant for users of cross-DSO CFI:

- There is a performance and code size overhead associated with each exported 
function, because each such function must have an associated jump table entry, 
which must be emitted even in the common case where the function is never 
address-taken anywhere in the program, and must be used even for direct calls 
between DSOs, in addition to the PLT overhead.

- There is no good way to take a CFI-valid address of a function written in 
assembly or a language not supported by Clang. The reason is that the code 
generator would need to insert a jump table in order to form a CFI-valid 
address for assembly functions, but there is no way in general for the code 
generator to determine the language of the function. This may be possible with 
LTO in the intra-DSO case, but in the cross-DSO case the only information 
available is the function declaration. One possible solution is to add a C 
wrapper for each assembly function, but these wrappers can present a 
significant maintenance burden for heavy users of assembly in addition to 
adding runtime overhead.

For these reasons, we provide the option of making the jump table non-canonical
with the flag ``-fno-sanitize-cfi-canonical-jump-tables``. When the jump
table is made non-canonical, symbol table entries point directly to the
function body. Any instances of a function's address being taken in C will
be replaced with a jump table address.

This scheme does have its own caveats, however. It does end up breaking
function address equality more aggressively than the default behavior,
especially in cross-DSO mode which normally preserves function address
equality entirely.

Furthermore, it is occasionally necessary for code not compiled with
``-fsanitize=cfi-icall`` to take a function address that is valid
for CFI. For example, this is necessary when a function's address
is taken by assembly code and then called by CFI-checking C code. The
``__attribute__((cfi_jump_table_canonical))`` attribute may be used to make
the jump table entry of a specific function canonical so that the external
code will end up taking a address for the function that will pass CFI checks.

Fixes PR41972.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65629

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/cfi-icall-canonical-jump-tables.c
  clang/test/CodeGen/cfi-icall-cross-dso.c
  clang/test/Driver/fsanitize.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-cfi-jump-table-canonical.cpp
  llvm/include/llvm/Transforms/IPO/LowerTypeTests.h
  llvm/lib/Transforms/IPO/CrossDSOCFI.cpp
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/test/Transforms/LowerTypeTests/import-icall.ll
  
llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll

Index: llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
===
--- /dev/null
+++ llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
@@ -0,0 +1,21 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t %s
+; RUN: llvm-modextract -b -n 1 -o - %t | llvm-dis | FileCheck %s
+
+; CHECK: !"f1", i8 1
+; CHECK: !"f2", i8 1
+; CHECK: !"f3", i8 0
+
+declare !type !1 void @f1()
+
+define void @f2() !type !1 {
+  ret void
+}
+
+define void @f3() "cfi-jump-table-canonical" !type !1 {
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"CFI Canonical Jump Tables", i32 0}
+!1 = !{i32 0, !"typeid1"}
Index: llvm/test/Transforms/LowerTypeTests/import-icall.ll
===
--- llvm/test/Transforms/LowerTypeTests/import-icall.ll
+++ llvm/test/Transforms/LowerTypeTests/import-icall.ll
@@ -3,6 +3,11 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = 

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

2019-08-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay marked an inline comment as done.
MaskRay added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5654-5656
 def warn_addition_in_bitshift : Warning<
   "operator '%0' has lower precedence than '%1'; "
+  "'%1' will be evaluated first">, InGroup, DefaultIgnore;

aaron.ballman wrote:
> MaskRay wrote:
> > aaron.ballman wrote:
> > > MaskRay wrote:
> > > > rsmith wrote:
> > > > > Do you have evidence that this warning has a significant 
> > > > > false-positive rate? In my experience it's very common for people to 
> > > > > think of `<<` as being a multiplication-like operator and be 
> > > > > surprised when it turns out to have lower precedence than addition.
> > > > warn_addition_in_bitshift has many false positives. Some results when 
> > > > searching for `[-Wshift-op-parentheses]` and the most common diagnostic 
> > > > `operator '<<' has lower precedence than '+'; '+' will be evaluated 
> > > > first`: 
> > > > 
> > > > https://gitship.com/srutscher/pdp-6-emulator/blob/f41b119eee2f409ea519b15f3a76cfecb70c03d8/emu/Makefile
> > > > https://www.openwall.com/lists/musl/2014/04/04/12
> > > > https://salmonella-freebsd-x86-64.call-cc.org/chicken-4/clang/freebsd/x86-64/2018/06/21/salmonella-report/install/bvsp-spline.html
> > > > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> > > > https://github.com/rdkit/rdkit/issues/145
> > > > ffmpeg 
> > > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?format=multiple=191743
> > > > https://clang.debian.net/logs/2015-03-25/fairymax_4.8v-1_unstable_clang.log
> > > > 
> > > Some of those look like true positives. For instance, the fix to 
> > > https://github.com/rdkit/rdkit/issues/145 was 
> > > https://github.com/rdkit/rdkit/commit/38ca41c8abc3f4429ae8df2ad79d3ff8b3fea0b6
> > >  which looks like the warning behaved as intended. 
> > > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> > >  doesn't really have anything to do with the diagnostic.
> > > 
> > > FWIW, in most of the cases, I feel like parens would clarify the code 
> > > (heck, they're already using parens in many of these cases). e.g.,
> > > ```
> > > fairymax.c:776:46: warning: operator '>>' has lower precedence than '+'; 
> > > '+' will be evaluated first [-Wshift-op-parentheses]
> > > MovesLeft = -(GamePtr+(Side==WHITE)>>1);
> > >   ~~~^~~~
> > > 
> > > dbvspis.c:606:20: warning: operator '<<' has lower precedence than '+'; 
> > > '+' will be evaluated first [-Wshift-op-parentheses]
> > > i3 = i2 + (*np + 1 << 1);
> > >^~~ ~~
> > > ```
> > > FWIW, I'm fine leaving this default on.
> > > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> > 
> > Note  -Wno-shift-op-parentheses
> > 
> > > https://github.com/rdkit/rdkit/issues/145 which looks like the warning 
> > > behaved as intended. 
> > 
> > Yes, this is a true positive. I just randomly searched for some cases, 
> > didn't carefully verify them, sorry.
> > 
> > > FWIW, in most of the cases, I feel like parens would clarify the code 
> > > (heck, they're already using parens in many of these cases). e.g.,
> > 
> > -Wparentheses certainly has its position and it does catch real errors. I 
> > also agree that many people are in favor of it. However, there are as many 
> > people who dislike it and add -Wno-parentheses to their build systems. Many 
> > people complain that extraneous parentheses clutter the code, especially 
> > multiple levels of parentheses.
> > 
> > The discrepancy between clang default and gcc default here is a bit 
> > unfortunate. People porting software to clang make a lot of style changes. 
> > They can mess up git blame, make bugfix backporting difficult, and so on.
> > 
> > The 3 subgroups of -Wparentheses have the most false positives. This is an 
> > indicator that they should not belong to the set of default-on warnings. 
> > People who use -Wall (a lot) will not notice the difference. I'm thinking 
> > if -Wall didn't include these controversial warnings people would be 
> > happier (clang -Wmost doesn't include -Wparentheses but unfortunately gcc 
> > doesn't have -Wmost). It is also unfortunate -Wparentheses is 
> > all-or-nothing...
> > -Wparentheses certainly has its position and it does catch real errors. I 
> > also agree that many people are in favor of it. However, there are as many 
> > people who dislike it and add -Wno-parentheses to their build systems. Many 
> > people complain that extraneous parentheses clutter the code, especially 
> > multiple levels of parentheses.
> 
> They don't seem to be complaining to our bug tracker -- I did not see any 
> complaints about the behavior of `warn_addition_in_bitshift` (there were a 
> few complaints about some of the other issues you're addressing in this patch 
> though).
> 
> My concern is that 

[PATCH] D65628: [clang-doc] Parallelize reducing phase

2019-08-01 Thread Diego Astiazarán via Phabricator via cfe-commits
DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: jakehehrlich, juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.

Reduce phase has been parallelized and a execution time was reduced by 60% with 
this.
The reading of bitcode (bitcode -> Info) was moved to this segment of code 
parallelized so it now happens just before reducing.


https://reviews.llvm.org/D65628

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -67,6 +68,12 @@
 llvm::cl::desc("CSS stylesheets to extend the default styles."),
 llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt ThreadCount(
+"thread-count",
+llvm::cl::desc("Threads to use for collecting and reducing infos."),
+llvm::cl::init(llvm::hardware_concurrency()),
+llvm::cl::cat(ClangDocCategory));
+
 enum OutputFormatTy {
   md,
   yaml,
@@ -153,30 +160,6 @@
   return Path;
 }
 
-// Iterate through tool results and build string map of info vectors from the
-// encoded bitstreams.
-bool bitcodeResultsToInfos(
-tooling::ToolResults ,
-llvm::StringMap>> ) {
-  bool Err = false;
-  Results.forEachResult([&](StringRef Key, StringRef Value) {
-llvm::BitstreamCursor Stream(Value);
-doc::ClangDocBitcodeReader Reader(Stream);
-auto Infos = Reader.readBitcode();
-if (!Infos) {
-  llvm::errs() << toString(Infos.takeError()) << "\n";
-  Err = true;
-  return;
-}
-for (auto  : Infos.get()) {
-  auto R =
-  Output.try_emplace(Key, std::vector>());
-  R.first->second.emplace_back(std::move(I));
-}
-  });
-  return Err;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -237,37 +220,67 @@
   // In ToolResults, the Key is the hashed USR and the value is the
   // bitcode-encoded representation of the Info object.
   llvm::outs() << "Collecting infos...\n";
-  llvm::StringMap>> USRToInfos;
-  if (bitcodeResultsToInfos(*Exec->get()->getToolResults(), USRToInfos))
-return 1;
+  llvm::StringMap> USRToBitcode;
+  Exec->get()->getToolResults()->forEachResult(
+  [&](StringRef Key, StringRef Value) {
+auto R = USRToBitcode.try_emplace(Key, std::vector());
+R.first->second.emplace_back(Value);
+  });
 
   // First reducing phase (reduce all decls into one info per decl).
-  llvm::outs() << "Reducing " << USRToInfos.size() << " infos...\n";
-  for (auto  : USRToInfos) {
-auto Reduced = doc::mergeInfos(Group.getValue());
-if (!Reduced) {
-  llvm::errs() << llvm::toString(Reduced.takeError());
-  continue;
-}
+  llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
+  bool Error = false;
+  llvm::ThreadPool Pool(ThreadCount);
+  for (auto  : USRToBitcode) {
+Pool.async([&]() {
+  std::vector> Infos;
 
-doc::Info *I = Reduced.get().get();
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
-  "." + Format);
-if (!InfoPath) {
-  llvm::errs() << toString(InfoPath.takeError()) << "\n";
-  return 1;
-}
-std::error_code FileErr;
-llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr, llvm::sys::fs::F_None);
-if (FileErr != OK) {
-  llvm::errs() << "Error opening info file: " << FileErr.message() << "\n";
-  continue;
-}
+  for (auto  : Group.getValue()) {
+llvm::BitstreamCursor Stream(Bitcode);
+doc::ClangDocBitcodeReader Reader(Stream);
+auto ReadInfos = Reader.readBitcode();
+if (!ReadInfos) {
+  llvm::errs() << toString(ReadInfos.takeError()) << "\n";
+  Error = true;
+  return;
+}
+std::move(ReadInfos->begin(), ReadInfos->end(),
+  std::back_inserter(Infos));
+  }
+
+  auto Reduced = doc::mergeInfos(Infos);
+  if (!Reduced) {
+llvm::errs() << llvm::toString(Reduced.takeError());
+return;
+  }
+
+  doc::Info *I = Reduced.get().get();
+  auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
+"." + Format);
+  if (!InfoPath) {
+llvm::errs() << toString(InfoPath.takeError()) << "\n";
+Error = true;
+return;
+  }
+  std::error_code FileErr;
+  llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
+  llvm::sys::fs::F_None);
+  if (FileErr != OK) {
+llvm::errs() << "Error 

[PATCH] D65627: [clang-doc] Continue after mapping error

2019-08-01 Thread Diego Astiazarán via Phabricator via cfe-commits
DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: jakehehrlich, juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.

The tool used to stop execution if there was an error in the mapping phase. It 
will now show the error but continue with the files that were mapped correctly.


https://reviews.llvm.org/D65627

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -228,10 +228,10 @@
   llvm::outs() << "Mapping decls...\n";
   auto Err =
   Exec->get()->execute(doc::newMapperActionFactory(CDCtx), ArgAdjuster);
-  if (Err) {
-llvm::errs() << toString(std::move(Err)) << "\n";
-return 1;
-  }
+  if (Err)
+llvm::errs() << "Error mapping decls in files. Clang-doc will ignore these 
"
+"files and continue:\n"
+ << toString(std::move(Err)) << "\n";
 
   // Collect values into output by key.
   // In ToolResults, the Key is the hashed USR and the value is the


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -228,10 +228,10 @@
   llvm::outs() << "Mapping decls...\n";
   auto Err =
   Exec->get()->execute(doc::newMapperActionFactory(CDCtx), ArgAdjuster);
-  if (Err) {
-llvm::errs() << toString(std::move(Err)) << "\n";
-return 1;
-  }
+  if (Err)
+llvm::errs() << "Error mapping decls in files. Clang-doc will ignore these "
+"files and continue:\n"
+ << toString(std::move(Err)) << "\n";
 
   // Collect values into output by key.
   // In ToolResults, the Key is the hashed USR and the value is the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65625: [clangd] Allow the client to specify multiple compilation databases in the 'initialize' request

2019-08-01 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

As background, some discussion of this took place on the mailing list a few 
months ago: http://lists.llvm.org/pipermail/clangd-dev/2019-January/000232.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65625



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


[PATCH] D65564: Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Seiya Nuta via Phabricator via cfe-commits
seiya accepted this revision.
seiya added a comment.

LGTM too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65564



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


[PATCH] D65625: [clangd] Allow the client to specify multiple compilation databases in the 'initialize' request

2019-08-01 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

This is useful for multi-project setups where each project has its own
compilation database, and particularly in cases where such databases
are located outside of the respective source directories (for example,
in build directories).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65625

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -39,7 +39,7 @@
 using ::testing::UnorderedElementsAre;
 
 TEST(GlobalCompilationDatabaseTest, FallbackCommand) {
-  DirectoryBasedGlobalCompilationDatabase DB(None);
+  DirectoryBasedGlobalCompilationDatabase DB(None, None);
   auto Cmd = DB.getFallbackCommand(testPath("foo/bar.cc"));
   EXPECT_EQ(Cmd.Directory, testPath("foo"));
   EXPECT_THAT(Cmd.CommandLine,
@@ -241,7 +241,7 @@
   // Note that gen2.cc goes missing with our following model, not sure this
   // happens in practice though.
   {
-DirectoryBasedGlobalCompilationDatabase DB(llvm::None);
+DirectoryBasedGlobalCompilationDatabase DB(llvm::None, llvm::None);
 std::vector DiscoveredFiles;
 auto Sub =
 DB.watch([](const std::vector Changes) {
@@ -263,7 +263,7 @@
 
   // With a custom compile commands dir.
   {
-DirectoryBasedGlobalCompilationDatabase DB(FS.Root.str().str());
+DirectoryBasedGlobalCompilationDatabase DB(FS.Root.str().str(), llvm::None);
 std::vector DiscoveredFiles;
 auto Sub =
 DB.watch([](const std::vector Changes) {
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -23,6 +23,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 
+#include "Path.h"
 #include "URI.h"
 #include "index/SymbolID.h"
 #include "clang/Index/IndexSymbol.h"
@@ -448,6 +449,14 @@
 };
 bool fromJSON(const llvm::json::Value &, ConfigurationSettings &);
 
+struct CompilationDatabasePath {
+  Path sourceDir;
+  Path dbPath;
+};
+bool fromJSON(const llvm::json::Value &, CompilationDatabasePath &);
+
+using CompilationDatabaseMap = std::vector;
+
 /// Clangd extension: parameters configurable at `initialize` time.
 /// LSP defines this type as `any`.
 struct InitializationOptions {
@@ -455,7 +464,12 @@
   // also set through the initialize request (initializationOptions field).
   ConfigurationSettings ConfigSettings;
 
-  llvm::Optional compilationDatabasePath;
+  llvm::Optional compilationDatabasePath;
+
+  // A map from source directories to directories containing compilation
+  // database files. Paths must be absolute.
+  llvm::Optional compilationDatabaseMap;
+
   // Additional flags to be included in the "fallback command" used when
   // the compilation database doesn't describe an opened file.
   // The command used will be approximately `clang $FILE $fallbackFlags`.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -931,6 +931,16 @@
   return true;
 }
 
+bool fromJSON(const llvm::json::Value , CompilationDatabasePath ) {
+  llvm::json::ObjectMapper O(Params);
+  if (!O)
+return true; // 'any' type in LSP.
+
+  O.map("sourceDir", P.sourceDir);
+  O.map("dbPath", P.dbPath);
+  return true;
+}
+
 bool fromJSON(const llvm::json::Value , InitializationOptions ) {
   llvm::json::ObjectMapper O(Params);
   if (!O)
@@ -938,6 +948,7 @@
 
   fromJSON(Params, Opts.ConfigSettings);
   O.map("compilationDatabasePath", Opts.compilationDatabasePath);
+  O.map("compilationDatabaseMap", Opts.compilationDatabaseMap);
   O.map("fallbackFlags", Opts.fallbackFlags);
   O.map("clangdFileStatus", Opts.FileStatus);
   return true;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -11,6 +11,7 @@
 
 #include "Function.h"
 #include "Path.h"
+#include "Protocol.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
@@ -66,7 +67,8 @@
 : public 

[PATCH] D64464: [CodeGen] Emit destructor calls for non-trivial C structs

2019-08-01 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 212942.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.

- Emit member access, compound literal, and call expressions as subexpressions 
of `ExprWithCleanups` if the expressions are of C struct types that require 
non-trivial destruction. This fixes the bug in IRGen where it was destructing 
the function return at the end of the enclosing scope rather than at the end of 
the full expression (see the changes made in test/CodeGenObjC/arc.m).
- Add compound literal expressions with automatic storage duration to the list 
of cleanup objects of `ExprWithCleanups` if the expressions have C struct types 
requiring non-trivial destruction. This enables IRGen to destruct the compound 
literals at the end of their enclosing scopes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64464

Files:
  include/clang/AST/ASTImporter.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/TextNodeDumper.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTImporter.cpp
  lib/AST/JSONNodeDumper.cpp
  lib/AST/TextNodeDumper.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGCleanup.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/AST/ast-dump-objc-arc-json.m
  test/AST/ast-dump-stmt.m
  test/CodeGenObjC/arc.m
  test/CodeGenObjC/strong-in-c-struct.m
  test/Import/objc-arc/Inputs/cleanup-objects.m
  test/Import/objc-arc/test-cleanup-object.m
  test/PCH/non-trivial-c-compound-literal.m
  test/SemaObjC/strong-in-c-struct.m
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -64,6 +64,10 @@
   llvm::cl::desc("The language to parse (default: c++)"),
   llvm::cl::init("c++"));
 
+static llvm::cl::opt
+ObjCARC("objc-arc", llvm::cl::init(false),
+llvm::cl::desc("Emable ObjC ARC"));
+
 static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false),
llvm::cl::desc("Dump combined AST"));
 
@@ -185,6 +189,8 @@
   Inv->getLangOpts()->ObjC = 1;
 }
   }
+  Inv->getLangOpts()->ObjCAutoRefCount = ObjCARC;
+
   Inv->getLangOpts()->Bool = true;
   Inv->getLangOpts()->WChar = true;
   Inv->getLangOpts()->Blocks = true;
Index: test/SemaObjC/strong-in-c-struct.m
===
--- test/SemaObjC/strong-in-c-struct.m
+++ test/SemaObjC/strong-in-c-struct.m
@@ -54,3 +54,21 @@
   func(^{ func2(x); });
   goto *ips; // expected-error {{cannot jump}}
 }
+
+void test_compound_literal0(int cond, id x) {
+  switch (cond) {
+  case 0:
+(void)(Strong){ .a = x }; // expected-note {{jump enters lifetime of a compound literal that is non-trivial to destruct}}
+break;
+  default: // expected-error {{cannot jump from switch statement to this case label}}
+break;
+  }
+}
+
+void test_compound_literal1(id x) {
+  static void *ips[] = { & };
+L0:  // expected-note {{possible target of indirect goto}}
+  ;
+  (void)(Strong){ .a = x }; // expected-note {{jump exits lifetime of a compound literal that is non-trivial to destruct}}
+  goto *ips; // expected-error {{cannot jump}}
+}
Index: test/PCH/non-trivial-c-compound-literal.m
===
--- /dev/null
+++ test/PCH/non-trivial-c-compound-literal.m
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -fobjc-arc -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -fobjc-arc -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+typedef struct {
+  id f;
+} S;
+
+static inline id getObj(id a) {
+  S *p = &(S){ .f = a };
+  return p->f;
+}
+
+#else
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+// CHECK: define internal i8* @getObj(
+// CHECK: %[[_COMPOUNDLITERAL:.*]] = alloca %[[STRUCT_S]],
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_S]]* %[[_COMPOUNDLITERAL]] to i8**
+// CHECK: call void @__destructor_8_s0(i8** %[[V5]])
+
+id test(id a) {
+  return getObj(a);
+}
+
+#endif
Index: test/Import/objc-arc/test-cleanup-object.m
===
--- /dev/null
+++ test/Import/objc-arc/test-cleanup-object.m
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -x objective-c -objc-arc -import %S/Inputs/cleanup-objects.m -dump-ast -expression %s | FileCheck %s
+
+// CHECK: FunctionDecl {{.*}} getObj 

Re: [PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-08-01 Thread Jan Korous via cfe-commits
Oh! Interesting! Thanks for investigating this.

I'm happy to review the patch.

> On Aug 1, 2019, at 5:17 PM, Puyan Lotfi  wrote:
> 
> Hi Jan, Thanks for the follow up!
> 
> Me and Compnerd narrowed down the issue to the inotify file count limit being 
> exceeded as the cause. DirectoryWatcherLinux::create() in 
> DirectoryWatcher-linux.cpp also doesn't properly perror, I'll post a patch 
> for this shortly to print perror info if the file count is exceeded.
> 
> The cause of the inotify limit being exceeded was... drumroll... _Dropbox_ 
> 
> PL
> 
> 
> 
> On Thu, Aug 1, 2019 at 11:24 AM Jan Korous via Phabricator via llvm-commits 
> mailto:llvm-comm...@lists.llvm.org>> wrote:
> jkorous added a comment.
> 
> Hi Puyan,
> 
> I failed to reproduce with llvm.org/master  
> (5faa533e47b0e54b04166b0257c5ebb48e6ffcaa 
>  >) on 
> Ubuntu 18.04.1 LTS both in debug and release build.
> 
> Since it sounds like you can reproduce "reliably" - can you please share more 
> info how to reproduce?
> 
> 
> Repository:
>   rL LLVM
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D58418/new/ 
> 
> https://reviews.llvm.org/D58418 
> 
> 
> 
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits 
> 

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


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

and the diff is shorter now :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



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


[PATCH] D65622: [clang-doc] Update documentation

2019-08-01 Thread Diego Astiazarán via Phabricator via cfe-commits
DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: jakehehrlich, juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.

HTML generator has been included in clang-tools-extra release notes.
clang-doc documentation file has been updated.


https://reviews.llvm.org/D65622

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-doc.rst


Index: clang-tools-extra/docs/clang-doc.rst
===
--- clang-tools-extra/docs/clang-doc.rst
+++ clang-tools-extra/docs/clang-doc.rst
@@ -17,7 +17,7 @@
 there.
 
 Use
-=
+===
 
 :program:`clang-doc` is a `LibTooling
 `_-based tool, and so requires a
@@ -25,19 +25,42 @@
 see `How To Setup Tooling For LLVM
 `_).
 
-The tool can be used on a single file or multiple files as defined in 
-the compile commands database:
+By default, the tool will run on all files listed in the given compile commands
+database:
 
 .. code-block:: console
 
-  $ clang-doc /path/to/file.cpp -p /path/to/compile/commands
+  $ clang-doc /path/to/compile_commands.json
 
-This generates an intermediate representation of the declarations and their
-associated information in the specified TUs, serialized to LLVM bitcode.
+The tool can also be used on a single file or multiple files if a build path is
+passed with the ``-p`` flag.
 
-As currently implemented, the tool is only able to parse TUs that can be 
-stored in-memory. Future additions will extend the current framework to use
-map-reduce frameworks to allow for use with large codebases.
+.. code-block:: console
+
+  $ clang-doc /path/to/file.cpp -p /path/to/build
+
+Output
+==
+
+:program:`clang-doc` produces a directory of documentation. One file is 
produced
+for each namespace and record in the project source code, containing all
+documentation (including contained functions, methods, and enums) for that 
item.
+
+The top-level directory is configurable through the ``output`` flag:
+
+.. code-block:: console
+
+  $ clang-doc -output=output/directory/ compile_commands.json
+
+Configuration
+=
+
+Configuration for :program:`clang-doc` is currently limited to command-line 
options.
+In the future, it may develop the ability to use a configuration file, but no 
such
+efforts are currently in progress.
+
+Options
+---
 
 :program:`clang-doc` offers the following options:
 
@@ -60,6 +83,11 @@
 -dump  - Dump intermediate results to bitcode file.
 -extra-arg=- Additional argument to append to the compiler 
command line
 -extra-arg-before= - Additional argument to prepend to the 
compiler command line
--omit-filenames- Omit filenames in output.
+--format=   - Format for outputted docs.
+  =yaml-   Documentation in YAML format.
+  =md  -   Documentation in MD format.
+  =html-   Documentation in HTML format.
 -output=   - Directory for outputting generated files.
 -p=- Build path
+--public   - Document only public declarations.
+--stylesheets= - CSS stylesheets to extend the default styles.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -52,7 +52,7 @@
 Improvements to clang-doc
 -
 
-The improvements are...
+- :doc:`clang-doc ` now generates documentation in HTML format.
 
 Improvements to clang-query
 ---


Index: clang-tools-extra/docs/clang-doc.rst
===
--- clang-tools-extra/docs/clang-doc.rst
+++ clang-tools-extra/docs/clang-doc.rst
@@ -17,7 +17,7 @@
 there.
 
 Use
-=
+===
 
 :program:`clang-doc` is a `LibTooling
 `_-based tool, and so requires a
@@ -25,19 +25,42 @@
 see `How To Setup Tooling For LLVM
 `_).
 
-The tool can be used on a single file or multiple files as defined in 
-the compile commands database:
+By default, the tool will run on all files listed in the given compile commands
+database:
 
 .. code-block:: console
 
-  $ clang-doc /path/to/file.cpp -p /path/to/compile/commands
+  $ clang-doc /path/to/compile_commands.json
 
-This generates an intermediate representation of the declarations and their
-associated information in the specified TUs, serialized to LLVM bitcode.
+The tool can also be used on a single file or multiple files if a build path is
+passed with the ``-p`` flag.
 
-As currently implemented, the tool is only able to parse TUs that can be 
-stored in-memory. Future additions will extend the current framework to use

[PATCH] D58531: [clang] Specify type of pthread_create builtin

2019-08-01 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D58531#1599209 , @probinson wrote:

> We've started running into this too in building the PS4 system. +jdoerfert 
> who added pthread_create to the builtin list.
>
> Looking at the patch, it seems straightforward enough although clearly needs 
> clang-format-diff run over it.
>  I don't touch Clang that much so I'm reluctant to okay it myself.


@probinson Thanks for making me aware of this patch.

> A separate point is whether it makes sense to be emitting this warning in the 
> first place for GE_Missing_type. I'd argue that, if we don't know the type of 
> the builtin, we should never emit the warning

@jrtc27 I hope the immediate need for this is gone after D58091 
 was finally committed? Given that I caused 
this mess I can take a look at the patch if you are still interested in it, are 
you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 212926.
yonghong-song added a comment.

reorder parameter arrayType to make it with default nullptr and simplifies the 
code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/builtin-preserve-access-index-array.c
  test/CodeGen/builtin-preserve-access-index.c

Index: test/CodeGen/builtin-preserve-access-index.c
===
--- test/CodeGen/builtin-preserve-access-index.c
+++ test/CodeGen/builtin-preserve-access-index.c
@@ -31,16 +31,16 @@
 }
 // CHECK: define dso_local i8* @unit4
 // CHECK-NOT: getelementptr
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 1)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[POINTER:[0-9]+]]
 
 const void *unit5(const int *arg[5]) {
   return _([1][2]);
 }
 // CHECK: define dso_local i8* @unit5
 // CHECK-NOT: getelementptr
-// CHECK: call i32** @llvm.preserve.array.access.index.p0p0i32.p0p0i32(i32** %{{[0-9a-z]+}}, i32 0, i32 1)
+// CHECK: call i32** @llvm.preserve.array.access.index.p0p0i32.p0p0i32(i32** %{{[0-9a-z]+}}, i32 0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 // CHECK-NOT: getelementptr
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[POINTER:[0-9]+]]
 
 struct s1 {
   char a;
@@ -141,7 +141,7 @@
 // CHECK: define dso_local i8* @unit13
 // CHECK: call %union.u* @llvm.preserve.struct.access.index.p0s_union.us.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S4:[0-9]+]]
 // CHECK: call %union.u* @llvm.preserve.union.access.index.p0s_union.us.p0s_union.us(%union.u* %{{[0-9a-z]+}}, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_I_U:[0-9]+]]
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 
 const void *unit14(union u3 *arg) {
   return _(>c.b[2]);
@@ -149,13 +149,13 @@
 // CHECK: define dso_local i8* @unit14
 // CHECK: call %union.u3* @llvm.preserve.union.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %{{[0-9a-z]+}}, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U3:[0-9]+]]
 // CHECK: call [4 x i32]* @llvm.preserve.struct.access.index.p0a4i32.p0s_struct.ss(%struct.s* %{{[0-9a-z]+}}, i32 0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_I_S:[0-9]+]]
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 
 const void *unit15(struct s4 *arg) {
   return _([2].c.a);
 }
 // CHECK: define dso_local i8* @unit15
-// CHECK: call %struct.s4* @llvm.preserve.array.access.index.p0s_struct.s4s.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 0, i32 2)
+// CHECK: call %struct.s4* @llvm.preserve.array.access.index.p0s_struct.s4s.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 0, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 // CHECK: call %union.u* @llvm.preserve.struct.access.index.p0s_union.us.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S4]]
 // CHECK: call %union.u* @llvm.preserve.union.access.index.p0s_union.us.p0s_union.us(%union.u* %{{[0-9a-z]+}}, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_I_U]]
 
@@ -163,15 +163,16 @@
   return _([2].a);
 }
 // CHECK: define dso_local i8* @unit16
-// CHECK: call %union.u3* @llvm.preserve.array.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %{{[0-9a-z]+}}, i32 0, i32 2)
+// CHECK: call %union.u3* @llvm.preserve.array.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %{{[0-9a-z]+}}, i32 0, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 // CHECK: call %union.u3* @llvm.preserve.union.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %{{[0-9a-z]+}}, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U3]]
 
+// CHECK: ![[POINTER]] = !DIDerivedType(tag: DW_TAG_pointer_type
+// CHECK: ![[STRUCT_S4]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s4"
+// CHECK: ![[UNION_I_U]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u"
+// CHECK: ![[UNION_U3]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u3"
+// 

r367632 - [DirectoryWatcher] Relax assumption to prevent test flakiness

2019-08-01 Thread Jan Korous via cfe-commits
Author: jkorous
Date: Thu Aug  1 16:24:30 2019
New Revision: 367632

URL: http://llvm.org/viewvc/llvm-project?rev=367632=rev
Log:
[DirectoryWatcher] Relax assumption to prevent test flakiness

Modified:
cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Modified: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp?rev=367632=367631=367632=diff
==
--- cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp (original)
+++ cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp Thu Aug  1 
16:24:30 2019
@@ -25,6 +25,24 @@ static void stopFSEventStream(FSEventStr
 
 namespace {
 
+/// This implementation is based on FSEvents API which implementation is
+/// aggressively coallescing events. This can manifest as duplicate events.
+///
+/// For example this scenario has been observed:
+///
+/// create foo/bar
+/// sleep 5 s
+/// create DirectoryWatcherMac for dir foo
+/// receive notification: bar EventKind::Modified
+/// sleep 5 s
+/// modify foo/bar
+/// receive notification: bar EventKind::Modified
+/// receive notification: bar EventKind::Modified
+/// sleep 5 s
+/// delete foo/bar
+/// receive notification: bar EventKind::Modified
+/// receive notification: bar EventKind::Modified
+/// receive notification: bar EventKind::Removed
 class DirectoryWatcherMac : public clang::DirectoryWatcher {
 public:
   DirectoryWatcherMac(

Modified: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp?rev=367632=367631=367632=diff
==
--- cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp (original)
+++ cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp Thu Aug  1 
16:24:30 2019
@@ -377,7 +377,7 @@ TEST(DirectoryWatcherTest, DeleteFile) {
   VerifyingConsumer TestConsumer{
   {{EventKind::Modified, "a"}},
   {{EventKind::Removed, "a"}},
-  {{EventKind::Modified, "a"}}};
+  {{EventKind::Modified, "a"}, {EventKind::Removed, "a"}}};
 
   auto DW = DirectoryWatcher::create(
   fixture.TestWatchedDir,


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


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:3402
  QualType eltType, bool inbounds,
- bool signedIndices, SourceLocation loc,
+ bool signedIndices, QualType *arrayType,
+ SourceLocation loc,

ast wrote:
> would it make sense to reorder and make it 'QualType *arrayType = nullptr', 
> so only explicit pointers would be passed and the rest will get 'nullptr' 
> automatically?
Yes, that makes sense. Previous I thought about grouping related fields 
together. But make nullptr as the default is a good idea.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



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


[PATCH] D65545: Handle some fs::remove failures

2019-08-01 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:647-649
+  if (std::error_code EC = llvm::sys::fs::remove(OF.TempFilename))
+getDiagnostics().Report(diag::err_fe_error_removing)
+  << OF.TempFilename << EC.message();

vsapsai wrote:
> Does the same logic as in ASTUnit.cpp apply here? I.e. if we failed to rename 
> a file and emitted a message about it, should we also have a message about 
> the failure to remove a file?
I've updated ASTUnit to be a bit better, and removed the logging here.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:1444-1445
   // Remove the file.
-  llvm::sys::fs::remove(File->path());
+  if ((EC = llvm::sys::fs::remove(File->path(
+break;
 

vsapsai wrote:
> Why are you interrupting the loop when cannot remove a file? Don't know which 
> option is the right one, just want to know your reasons.
The loops already bail when `EC` is set, and here I figure if we can't remove 
the base file we shouldn't try to remove its corresponding timestamp.



Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:935-936
   // Remove the old index file. It isn't relevant any more.
-  llvm::sys::fs::remove(IndexPath);
+  if (std::error_code EC = llvm::sys::fs::remove(IndexPath))
+return llvm::createStringError(EC, "failed removing \"" + IndexPath + 
"\"");
 

vsapsai wrote:
> Don't have a strong opinion but it looks odd that here in `createStringError` 
> you are using string concatenation and a few lines lower `%s`.
Yeah this part of `createStringError` bugs me... It only takes `const char*` 
for `%s`, and here I have a `Twine`.  I changed it to `%s`.



Comment at: llvm/lib/Support/LockFileManager.cpp:58
+if (std::error_code EC = sys::fs::remove(LockFileName))
+  report_fatal_error("Unable to remove invalid lock file \"" + 
LockFileName + "\": " + EC.message());
 return None;

vsapsai wrote:
> Do you plan to keep using `report_fatal_error` in `LockFileManager`? It 
> should help with discovering problems with modules but making it a fatal 
> error forever seems a little bit scary.
For some of the other destructors I was thinking that failing to remove a file 
could be non-fatal, but for lock files we can't really do much if they stay 
around. I think those are probably better as fatal than any other one. That, or 
return `Expected`, but combining with `Optional>` 
is... ew...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545



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


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

The corresponding IR intrinsic interface change is at 
https://reviews.llvm.org/D65617
The BPF backend to utilize this information is at 
https://reviews.llvm.org/D65618


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



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


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:3402
  QualType eltType, bool inbounds,
- bool signedIndices, SourceLocation loc,
+ bool signedIndices, QualType *arrayType,
+ SourceLocation loc,

would it make sense to reorder and make it 'QualType *arrayType = nullptr', so 
only explicit pointers would be passed and the rest will get 'nullptr' 
automatically?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



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


[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

2019-08-01 Thread Romain Geissler via Phabricator via cfe-commits
Romain-Geissler-1A created this revision.
Romain-Geissler-1A added reviewers: serge-sans-paille, chandlerc.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

This simple patch ignores -fsemantic-interposition/-fno-semantic-interposition 
that may be used by some gcc users, by copy/pasting what was done for other 
similar -f flags.

Cheers,
Romain


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65616

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -298,6 +298,7 @@
 // RUN: -fno-implement-inlines -fimplement-inlines\
 // RUN: -fstack-check \
 // RUN: -fforce-addr  \
+// RUN: -fno-semantic-interposition   \
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3153,6 +3153,7 @@
 defm devirtualize : BooleanFFlag<"devirtualize">, 
Group;
 defm devirtualize_speculatively : BooleanFFlag<"devirtualize-speculatively">,
 Group;
+defm semantic_interposition : BooleanFFlag<"semantic-interposition">, 
Group;
 
 // Generic gfortran options.
 def A_DASH : Joined<["-"], "A-">, Group;


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -298,6 +298,7 @@
 // RUN: -fno-implement-inlines -fimplement-inlines\
 // RUN: -fstack-check \
 // RUN: -fforce-addr  \
+// RUN: -fno-semantic-interposition   \
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3153,6 +3153,7 @@
 defm devirtualize : BooleanFFlag<"devirtualize">, Group;
 defm devirtualize_speculatively : BooleanFFlag<"devirtualize-speculatively">,
 Group;
+defm semantic_interposition : BooleanFFlag<"semantic-interposition">, Group;
 
 // Generic gfortran options.
 def A_DASH : Joined<["-"], "A-">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

2019-08-01 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song created this revision.
yonghong-song added a reviewer: ast.
Herald added subscribers: kristina, arphaman, aprantl.
Herald added a project: clang.

Previously, debuginfo types are annotated to
IR builtin preserve_struct_access_index() and
preserve_union_access_index(), but not
preserve_array_access_index(). The debug info
is useful to identify the root type name which
later will be used for type comparison.

For user access without explicit type conversions,
the previous scheme works as we can ignore intermediate
compiler generated type conversions (e.g., from union types to
union members) and still generate correct access index string.

The issue comes with user explicit type conversions, e.g.,
converting an array to a structure like below:

  struct t { int a; char b[40]; };
  struct p { int c; int d; };
  struct t *var = ...;
  ... __builtin_preserve_access_index(&(((struct p *)&(var->b[0]))->d)) ...

Although BPF backend can derive the type of &(var->b[0]),
explicit type annotation make checking more consistent
and less error prone.

Another benefit is for multiple dimension array handling.
For example,

  struct p { int c; int d; } g[8][9][10];
  ... __builtin_preserve_access_index([2][3][4].d) ...

It would be possible to calculate the number of "struct p"'s
before accessing its member "d" if array debug info is
available as it contains each dimension range.

This patch enables to annotate IR builtin preserve_array_access_index()
with either debuginfo array type or pointer type,
depending on user expression.


Repository:
  rC Clang

https://reviews.llvm.org/D65615

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/builtin-preserve-access-index-array.c
  test/CodeGen/builtin-preserve-access-index.c

Index: test/CodeGen/builtin-preserve-access-index.c
===
--- test/CodeGen/builtin-preserve-access-index.c
+++ test/CodeGen/builtin-preserve-access-index.c
@@ -31,16 +31,16 @@
 }
 // CHECK: define dso_local i8* @unit4
 // CHECK-NOT: getelementptr
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 1)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[POINTER:[0-9]+]]
 
 const void *unit5(const int *arg[5]) {
   return _([1][2]);
 }
 // CHECK: define dso_local i8* @unit5
 // CHECK-NOT: getelementptr
-// CHECK: call i32** @llvm.preserve.array.access.index.p0p0i32.p0p0i32(i32** %{{[0-9a-z]+}}, i32 0, i32 1)
+// CHECK: call i32** @llvm.preserve.array.access.index.p0p0i32.p0p0i32(i32** %{{[0-9a-z]+}}, i32 0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 // CHECK-NOT: getelementptr
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %{{[0-9a-z]+}}, i32 0, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[POINTER:[0-9]+]]
 
 struct s1 {
   char a;
@@ -141,7 +141,7 @@
 // CHECK: define dso_local i8* @unit13
 // CHECK: call %union.u* @llvm.preserve.struct.access.index.p0s_union.us.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S4:[0-9]+]]
 // CHECK: call %union.u* @llvm.preserve.union.access.index.p0s_union.us.p0s_union.us(%union.u* %{{[0-9a-z]+}}, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_I_U:[0-9]+]]
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 
 const void *unit14(union u3 *arg) {
   return _(>c.b[2]);
@@ -149,13 +149,13 @@
 // CHECK: define dso_local i8* @unit14
 // CHECK: call %union.u3* @llvm.preserve.union.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %{{[0-9a-z]+}}, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U3:[0-9]+]]
 // CHECK: call [4 x i32]* @llvm.preserve.struct.access.index.p0a4i32.p0s_struct.ss(%struct.s* %{{[0-9a-z]+}}, i32 0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_I_S:[0-9]+]]
-// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2)
+// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %{{[0-9a-z]+}}, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index !{{[0-9]+}}
 
 const void *unit15(struct s4 *arg) {
   return _([2].c.a);
 }
 // CHECK: define dso_local i8* @unit15
-// CHECK: call %struct.s4* @llvm.preserve.array.access.index.p0s_struct.s4s.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 0, i32 2)
+// CHECK: call %struct.s4* @llvm.preserve.array.access.index.p0s_struct.s4s.p0s_struct.s4s(%struct.s4* %{{[0-9a-z]+}}, i32 0, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index 

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

2019-08-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Looks good to me. I was hoping Richard would take a look, but I reproduced the 
crash, and I'd rather see the fix land sooner than later. Thanks!


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

https://reviews.llvm.org/D61027



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


[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

2019-08-01 Thread Rong Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
xur marked an inline comment as done.
Closed by commit rL367628: [PGO] Add PGO support at -O0 in the experimental new 
pass manager (authored by xur, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64029?vs=212237=212918#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64029

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  cfe/trunk/test/Profile/gcc-flag-compatibility.c
  llvm/trunk/include/llvm/Passes/PassBuilder.h
  llvm/trunk/lib/Passes/PassBuilder.cpp
  llvm/trunk/test/Other/new-pm-pgo-O0.ll

Index: llvm/trunk/include/llvm/Passes/PassBuilder.h
===
--- llvm/trunk/include/llvm/Passes/PassBuilder.h
+++ llvm/trunk/include/llvm/Passes/PassBuilder.h
@@ -629,6 +629,12 @@
 TopLevelPipelineParsingCallbacks.push_back(C);
   }
 
+  /// Add PGOInstrumenation passes for O0 only.
+  void addPGOInstrPassesForO0(ModulePassManager , bool DebugLogging,
+  bool RunProfileGen, bool IsCS,
+  std::string ProfileFile,
+  std::string ProfileRemappingFile);
+
 private:
   static Optional>
   parsePipelineText(StringRef Text);
@@ -660,7 +666,6 @@
  OptimizationLevel Level, bool RunProfileGen, bool IsCS,
  std::string ProfileFile,
  std::string ProfileRemappingFile);
-
   void invokePeepholeEPCallbacks(FunctionPassManager &, OptimizationLevel);
 
   // Extension Point callbacks
Index: llvm/trunk/test/Other/new-pm-pgo-O0.ll
===
--- llvm/trunk/test/Other/new-pm-pgo-O0.ll
+++ llvm/trunk/test/Other/new-pm-pgo-O0.ll
@@ -0,0 +1,21 @@
+; RUN: opt -debug-pass-manager -passes='default' -pgo-kind=pgo-instr-gen-pipeline -profile-file='temp' %s 2>&1 |FileCheck %s --check-prefixes=GEN
+; RUN: llvm-profdata merge %S/Inputs/new-pm-pgo.proftext -o %t.profdata
+; RUN: opt -debug-pass-manager -passes='default' -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 |FileCheck %s --check-prefixes=USE_DEFAULT,USE
+; RUN: opt -debug-pass-manager -passes='thinlto-pre-link' -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 \
+; RUN: |FileCheck %s --check-prefixes=USE_PRE_LINK,USE
+; RUN: opt -debug-pass-manager -passes='lto-pre-link' -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 \
+; RUN: |FileCheck %s --check-prefixes=USE_PRE_LINK,USE
+; RUN: opt -debug-pass-manager -passes='thinlto' -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 \
+; RUN: |FileCheck %s --check-prefixes=USE_POST_LINK,USE
+
+;
+; GEN: Running pass: PGOInstrumentationGen
+; USE_DEFAULT: Running pass: PGOInstrumentationUse
+; USE_PRE_LINK: Running pass: PGOInstrumentationUse
+; USE_POST_LINK-NOT: Running pass: PGOInstrumentationUse
+; USE-NOT: Running pass: PGOIndirectCallPromotion
+; USE-NOT: Running pass: PGOMemOPSizeOpt
+
+define void @foo() {
+  ret void
+}
Index: llvm/trunk/lib/Passes/PassBuilder.cpp
===
--- llvm/trunk/lib/Passes/PassBuilder.cpp
+++ llvm/trunk/lib/Passes/PassBuilder.cpp
@@ -541,6 +541,7 @@
 bool RunProfileGen, bool IsCS,
 std::string ProfileFile,
 std::string ProfileRemappingFile) {
+  assert(Level != O0 && "Not expecting O0 here!");
   // Generally running simplification passes and the inliner with an high
   // threshold results in smaller executables, but there may be cases where
   // the size grows, so let's be conservative here and skip this simplification
@@ -571,34 +572,62 @@
 CGPipeline.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM)));
 
 MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPipeline)));
+
+// Delete anything that is now dead to make sure that we don't instrument
+// dead code. Instrumentation can end up keeping dead code around and
+// dramatically increase code size.
+MPM.addPass(GlobalDCEPass());
   }
 
-  // Delete anything that is now dead to make sure that we don't instrument
-  // dead code. Instrumentation can end up keeping dead code around and
-  // dramatically increase code size.
-  MPM.addPass(GlobalDCEPass());
+  if (!RunProfileGen) {
+assert(!ProfileFile.empty() && "Profile use expecting a profile file!");
+MPM.addPass(PGOInstrumentationUse(ProfileFile, ProfileRemappingFile, IsCS));
+// Cache ProfileSummaryAnalysis once to avoid the potential need to insert
+// RequireAnalysisPass for PSI before subsequent non-module passes.
+MPM.addPass(RequireAnalysisPass());
+return;
+  }
 
-  if 

r367628 - [PGO] Add PGO support at -O0 in the experimental new pass manager

2019-08-01 Thread Rong Xu via cfe-commits
Author: xur
Date: Thu Aug  1 15:36:34 2019
New Revision: 367628

URL: http://llvm.org/viewvc/llvm-project?rev=367628=rev
Log:
[PGO] Add PGO support at -O0 in the experimental new pass manager

Add PGO support at -O0 in the experimental new pass manager to sync the
behavior of the legacy pass manager.

Also change the test of gcc-flag-compatibility.c for more complete test:
(1) change the match string to "profc" and "profd" to ensure the
instrumentation is happening.
(2) add IR format proftext so that PGO use compilation is tested.

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

Added:
cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/Profile/gcc-flag-compatibility.c

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=367628=367627=367628=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Aug  1 15:36:34 2019
@@ -1117,6 +1117,16 @@ void EmitAssemblyHelper::EmitAssemblyWit
   // code generation.
   MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));
 
+  // At -O0, we can still do PGO. Add all the requested passes for
+  // instrumentation PGO, if requested.
+  if (PGOOpt && (PGOOpt->Action == PGOOptions::IRInstr ||
+ PGOOpt->Action == PGOOptions::IRUse))
+PB.addPGOInstrPassesForO0(
+MPM, CodeGenOpts.DebugPassManager,
+/* RunProfileGen */ (PGOOpt->Action == PGOOptions::IRInstr),
+/* IsCS */ false, PGOOpt->ProfileFile,
+PGOOpt->ProfileRemappingFile);
+
   // At -O0 we directly run necessary sanitizer passes.
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
 MPM.addPass(createModuleToFunctionPassAdaptor(BoundsCheckingPass()));

Added: cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext?rev=367628=auto
==
--- cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext (added)
+++ cfe/trunk/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext Thu Aug  1 
15:36:34 2019
@@ -0,0 +1,11 @@
+# IR level Instrumentation Flag
+:ir
+main
+# Func Hash:
+34137660316
+# Num Counters:
+2
+# Counter Values:
+100
+1
+

Modified: cfe/trunk/test/Profile/gcc-flag-compatibility.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/gcc-flag-compatibility.c?rev=367628=367627=367628=diff
==
--- cfe/trunk/test/Profile/gcc-flag-compatibility.c (original)
+++ cfe/trunk/test/Profile/gcc-flag-compatibility.c Thu Aug  1 15:36:34 2019
@@ -7,26 +7,50 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate | FileCheck 
-check-prefix=PROFILE-GEN %s
-// PROFILE-GEN: __llvm_profile_filename
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
+// PROFILE-GEN: @__profc_main = private global [2 x i64] zeroinitializer, 
section "__llvm_prf_cnts", align 8
+// PROFILE-GEN: @__profd_main = private global
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to | 
FileCheck -check-prefix=PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RxUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
 // PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|\\5C}}{{.*}}\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata
+// This uses Clang FE format profile.
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir/some/path
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/default.profdata
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path | FileCheck -check-prefix=PROFILE-USE-2 %s
-// PROFILE-USE-2: = !{!"branch_weights", i32 101, i32 2}
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE %s
+// RUN: %clang %s -o - -Xclang 

[PATCH] D65545: Handle some fs::remove failures

2019-08-01 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:107-111
 TemporaryFiles::~TemporaryFiles() {
   llvm::MutexGuard Guard(Mutex);
   for (const auto  : Files)
-llvm::sys::fs::remove(File.getKey());
+if (std::error_code EC = llvm::sys::fs::remove(File.getKey()))
+  llvm::report_fatal_error("failed removing file \"" + File.getKey() + 
"\": " + EC.message());

vsapsai wrote:
> Clangd uses `PrecompiledPreamble` but not `TemporaryFiles` as far as I can 
> tell. `report_fatal_error` can be really disruptive for clangd, so it would 
> be good to get an opinion from somebody working on it if this change would 
> impact them.
Since client code in general might have different error handling/reporting 
strategy and can't do much about failure in destructor here, I'd consider just 
some kind of logging or assert here. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545



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


[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

2019-08-01 Thread Rong Xu via Phabricator via cfe-commits
xur marked 3 inline comments as done.
xur added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1125
+PB.addPGOInstrPassesForO0(MPM, CodeGenOpts.DebugPassManager,
+  /* RunProfileGen */ PGOOpt->Action ==
+  PGOOptions::IRInstr,

chandlerc wrote:
> Minor nit, here and else where with the comment-named bools I'd use the 
> style: `/*RunProfileGen=*/PGOOpt->Action == PGOOPtions::IRInstr`
> 
> This was suggested as more consistent w/ Clang style, and it seems a bit 
> cleaner. Also, clang-tidy will recognize and check that the comment uses the 
> same name as the parameter.
Thanks for the suggestion and the reasoning.

I actually wrote it in one line.
This was produced by "clang/tools/clang-format/clang-format-diff.py". :-)

I'm using now "/*RunProfileGen=*/ (PGOOpt->Action == PGOOPtions::IRInstr)" so 
clang-format-diff.py won't complain



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

https://reviews.llvm.org/D64029



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


r367622 - Fix Windows branch of FileManagerTest changes

2019-08-01 Thread Harlan Haskins via cfe-commits
Author: harlanhaskins
Date: Thu Aug  1 14:58:56 2019
New Revision: 367622

URL: http://llvm.org/viewvc/llvm-project?rev=367622=rev
Log:
Fix Windows branch of FileManagerTest changes

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=367622=367621=367622=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Aug  1 14:58:56 2019
@@ -163,7 +163,7 @@ TEST_F(FileManagerTest, getFileReturnsVa
   file = manager.getFile(FileName);
   ASSERT_TRUE(file);
 
-  dir = file->getDir();
+  dir = (*file)->getDir();
   ASSERT_TRUE(dir != NULL);
   EXPECT_EQ(DirName, dir->getName());
 #endif


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


r367620 - Fix use-after-move in ClangBasicTests

2019-08-01 Thread Harlan Haskins via cfe-commits
Author: harlanhaskins
Date: Thu Aug  1 14:50:16 2019
New Revision: 367620

URL: http://llvm.org/viewvc/llvm-project?rev=367620=rev
Log:
Fix use-after-move in ClangBasicTests

Modified:
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=367620=367619=367620=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Aug  1 14:50:16 2019
@@ -212,6 +212,7 @@ TEST_F(FileManagerTest, getFileReturnsEr
   auto statCache = llvm::make_unique();
   statCache->InjectDirectory(".", 41);
   statCache->InjectFile("foo.cpp", 42);
+  statCache->InjectDirectory("MyDirectory", 49);
   manager.setStatCache(std::move(statCache));
 
   // Create a virtual bar.cpp file.
@@ -221,7 +222,6 @@ TEST_F(FileManagerTest, getFileReturnsEr
   ASSERT_FALSE(file);
   ASSERT_EQ(file.getError(), std::errc::no_such_file_or_directory);
 
-  statCache->InjectDirectory("MyDirectory", 49);
   auto readingDirAsFile = manager.getFile("MyDirectory");
   ASSERT_FALSE(readingDirAsFile);
   ASSERT_EQ(readingDirAsFile.getError(), std::errc::is_a_directory);


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


[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

2019-08-01 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM with two nits addressed, thanks!




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1125
+PB.addPGOInstrPassesForO0(MPM, CodeGenOpts.DebugPassManager,
+  /* RunProfileGen */ PGOOpt->Action ==
+  PGOOptions::IRInstr,

Minor nit, here and else where with the comment-named bools I'd use the style: 
`/*RunProfileGen=*/PGOOpt->Action == PGOOPtions::IRInstr`

This was suggested as more consistent w/ Clang style, and it seems a bit 
cleaner. Also, clang-tidy will recognize and check that the comment uses the 
same name as the parameter.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1839
+addPGOInstrPassesForO0(MPM, DebugLogging,
+   /* RunProfileGen */
+   PGOOpt->Action == PGOOptions::IRInstr,

Same nit about the parameter comments here.


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

https://reviews.llvm.org/D64029



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


[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367618: [clang] Change FileManager to use llvm::ErrorOr 
instead of null on failure (authored by harlanhaskins, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65534?vs=212860=212907#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65534

Files:
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp


Index: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }


Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r367617 - [clang-tools-extra] Adopt FileManager's error-returning APIs

2019-08-01 Thread Harlan Haskins via cfe-commits
Author: harlanhaskins
Date: Thu Aug  1 14:32:01 2019
New Revision: 367617

URL: http://llvm.org/viewvc/llvm-project?rev=367617=rev
Log:
[clang-tools-extra] Adopt FileManager's error-returning APIs

The FileManager has been updated to return llvm::ErrorOr from getFile
and getDirectory, this commit updates all the callers of those APIs from
clang.

Modified:

clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/trunk/clang-change-namespace/tool/ClangChangeNamespace.cpp
clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/clang-move/Move.cpp
clang-tools-extra/trunk/clang-move/tool/ClangMove.cpp
clang-tools-extra/trunk/clang-reorder-fields/tool/ClangReorderFields.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp

Modified: 
clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp?rev=367617=367616=367617=diff
==
--- 
clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
 Thu Aug  1 14:32:01 2019
@@ -151,13 +151,13 @@ groupReplacements(const TUReplacements &
   auto AddToGroup = [&](const tooling::Replacement , bool FromDiag) {
 // Use the file manager to deduplicate paths. FileEntries are
 // automatically canonicalized.
-if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) 
{
+if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (FromDiag) {
-auto  = DiagReplacements[Entry];
+auto  = DiagReplacements[*Entry];
 if (!Replaces.insert(R).second)
   return;
   }
-  GroupedReplacements[Entry].push_back(R);
+  GroupedReplacements[*Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {
   errs() << "Described file '" << R.getFilePath()
  << "' doesn't exist. Ignoring...\n";

Modified: 
clang-tools-extra/trunk/clang-change-namespace/tool/ClangChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-change-namespace/tool/ClangChangeNamespace.cpp?rev=367617=367616=367617=diff
==
--- 
clang-tools-extra/trunk/clang-change-namespace/tool/ClangChangeNamespace.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-change-namespace/tool/ClangChangeNamespace.cpp 
Thu Aug  1 14:32:01 2019
@@ -147,8 +147,8 @@ int main(int argc, const char **argv) {
   for (auto I = ChangedFiles.begin(), E = ChangedFiles.end(); I != E; ++I) 
{
 OS << "  {\n";
 OS << "\"FilePath\": \"" << *I << "\",\n";
-const auto *Entry = FileMgr.getFile(*I);
-auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
+const auto Entry = FileMgr.getFile(*I);
+auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
 std::string Content;
 llvm::raw_string_ostream ContentStream(Content);
 Rewrite.getEditBuffer(ID).write(ContentStream);
@@ -165,9 +165,9 @@ int main(int argc, const char **argv) {
   }
 
   for (const auto  : ChangedFiles) {
-const auto *Entry = FileMgr.getFile(File);
+const auto Entry = FileMgr.getFile(File);
 
-auto ID = Sources.getOrCreateFileID(Entry, SrcMgr::C_User);
+auto ID = Sources.getOrCreateFileID(*Entry, SrcMgr::C_User);
 outs() << "== " << File << " ==\n";
 Rewrite.getEditBuffer(ID).write(llvm::outs());
 outs() << "\n\n";

Modified: clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp?rev=367617=367616=367617=diff
==
--- clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp Thu Aug  1 
14:32:01 2019
@@ -306,8 +306,7 @@ std::string IncludeFixerSemaSource::mini
 
   // Get the FileEntry for the include.
   StringRef StrippedInclude = Include.trim("\"<>");
-  const FileEntry *Entry =
-  SourceManager.getFileManager().getFile(StrippedInclude);
+  auto Entry = SourceManager.getFileManager().getFile(StrippedInclude);
 
   // If the file doesn't exist return the path from the database.
   // FIXME: This should never happen.
@@ -316,7 +315,7 @@ std::string 

r367616 - [clang] Adopt new FileManager error-returning APIs

2019-08-01 Thread Harlan Haskins via cfe-commits
Author: harlanhaskins
Date: Thu Aug  1 14:31:56 2019
New Revision: 367616

URL: http://llvm.org/viewvc/llvm-project?rev=367616=rev
Log:
[clang] Adopt new FileManager error-returning APIs

Update the callers of FileManager::getFile and FileManager::getDirectory to 
handle the new llvm::ErrorOr-returning methods.

Signed-off-by: Harlan Haskins 

Modified:
cfe/trunk/lib/ARCMigrate/FileRemapper.cpp
cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/Basic/Module.cpp
cfe/trunk/lib/Basic/SourceManager.cpp
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
cfe/trunk/lib/Frontend/TextDiagnostic.cpp
cfe/trunk/lib/Lex/HeaderMap.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/PPLexerChange.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/lib/Tooling/Refactoring.cpp
cfe/trunk/tools/clang-format/ClangFormat.cpp
cfe/trunk/tools/clang-import-test/clang-import-test.cpp
cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
cfe/trunk/tools/clang-refactor/TestSupport.cpp
cfe/trunk/tools/clang-rename/ClangRename.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/Indexing.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
cfe/trunk/unittests/Lex/PPCallbacksTest.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
cfe/trunk/unittests/Tooling/RewriterTestContext.h

Modified: cfe/trunk/lib/ARCMigrate/FileRemapper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/FileRemapper.cpp?rev=367616=367615=367616=diff
==
--- cfe/trunk/lib/ARCMigrate/FileRemapper.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/FileRemapper.cpp Thu Aug  1 14:31:56 2019
@@ -78,26 +78,26 @@ bool FileRemapper::initFromFile(StringRe
 Diag);
 StringRef toFilename = lines[idx+2];
 
-const FileEntry *origFE = FileMgr->getFile(fromFilename);
+llvm::ErrorOr origFE = FileMgr->getFile(fromFilename);
 if (!origFE) {
   if (ignoreIfFilesChanged)
 continue;
   return report("File does not exist: " + fromFilename, Diag);
 }
-const FileEntry *newFE = FileMgr->getFile(toFilename);
+llvm::ErrorOr newFE = FileMgr->getFile(toFilename);
 if (!newFE) {
   if (ignoreIfFilesChanged)
 continue;
   return report("File does not exist: " + toFilename, Diag);
 }
 
-if ((uint64_t)origFE->getModificationTime() != timeModified) {
+if ((uint64_t)(*origFE)->getModificationTime() != timeModified) {
   if (ignoreIfFilesChanged)
 continue;
   return report("File was modified: " + fromFilename, Diag);
 }
 
-pairs.push_back(std::make_pair(origFE, newFE));
+pairs.push_back(std::make_pair(*origFE, *newFE));
   }
 
   for (unsigned i = 0, e = pairs.size(); i != e; ++i)
@@ -152,9 +152,11 @@ bool FileRemapper::flushToFile(StringRef
   newOut.write(mem->getBufferStart(), mem->getBufferSize());
   newOut.close();
 
-  const FileEntry *newE = FileMgr->getFile(tempPath);
-  remap(origFE, newE);
-  infoOut << newE->getName() << '\n';
+  auto newE = FileMgr->getFile(tempPath);
+  if (newE) {
+remap(origFE, *newE);
+infoOut << (*newE)->getName() << '\n';
+  }
 }
   }
 
@@ -224,7 +226,9 @@ void FileRemapper::remap(const FileEntry
 }
 
 const FileEntry *FileRemapper::getOriginalFile(StringRef filePath) {
-  const FileEntry *file = FileMgr->getFile(filePath);
+  const FileEntry *file = nullptr;
+  if (auto fileOrErr = FileMgr->getFile(filePath))
+file = *fileOrErr;
   // If we are updating a file that overridden an original file,
   // actually update the original file.
   llvm::DenseMap::iterator

Modified: cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/ObjCMT.cpp?rev=367616=367615=367616=diff
==
--- cfe/trunk/lib/ARCMigrate/ObjCMT.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/ObjCMT.cpp Thu Aug  1 14:31:56 2019
@@ -2141,10 +2141,11 @@ private:
   StringRef Val = ValueString->getValue(ValueStorage);
 
   if (Key == "file") {
-const FileEntry *FE = FileMgr.getFile(Val);
-if (!FE)
+auto FE = FileMgr.getFile(Val);
+if (FE)
+  Entry.File = *FE;
+else
   Ignore = true;
-  

r367615 - [clang] Adopt llvm::ErrorOr in FileManager methods

2019-08-01 Thread Harlan Haskins via cfe-commits
Author: harlanhaskins
Date: Thu Aug  1 14:31:49 2019
New Revision: 367615

URL: http://llvm.org/viewvc/llvm-project?rev=367615=rev
Log:
[clang] Adopt llvm::ErrorOr in FileManager methods

Previously, the FileManager would use NULL returns to signify whether a file 
existed, but that doesn’t cover permissions issues or anything else that might 
occur while trying to stat or read a file. Instead, convert getFile and 
getDirectory into returning llvm::ErrorOr

Signed-off-by: Harlan Haskins 

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=367615=367614=367615=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Thu Aug  1 14:31:49 2019
@@ -132,20 +132,24 @@ class FileManager : public RefCountedBas
   SmallVector, 4> VirtualFileEntries;
 
   /// A cache that maps paths to directory entries (either real or
-  /// virtual) we have looked up
+  /// virtual) we have looked up, or an error that occurred when we looked up
+  /// the directory.
   ///
   /// The actual Entries for real directories/files are
   /// owned by UniqueRealDirs/UniqueRealFiles above, while the Entries
   /// for virtual directories/files are owned by
   /// VirtualDirectoryEntries/VirtualFileEntries above.
   ///
-  llvm::StringMap SeenDirEntries;
+  llvm::StringMap, llvm::BumpPtrAllocator>
+  SeenDirEntries;
 
   /// A cache that maps paths to file entries (either real or
-  /// virtual) we have looked up.
+  /// virtual) we have looked up, or an error that occurred when we looked up
+  /// the file.
   ///
   /// \see SeenDirEntries
-  llvm::StringMap SeenFileEntries;
+  llvm::StringMap, llvm::BumpPtrAllocator>
+  SeenFileEntries;
 
   /// The canonical names of directories.
   llvm::DenseMap CanonicalDirNames;
@@ -164,8 +168,9 @@ class FileManager : public RefCountedBas
   // Caching.
   std::unique_ptr StatCache;
 
-  bool getStatValue(StringRef Path, llvm::vfs::Status , bool isFile,
-std::unique_ptr *F);
+  std::error_code getStatValue(StringRef Path, llvm::vfs::Status ,
+   bool isFile,
+   std::unique_ptr *F);
 
   /// Add all ancestors of the given path (pointing to either a file
   /// or a directory) as virtual directories.
@@ -198,24 +203,27 @@ public:
   /// Lookup, cache, and verify the specified directory (real or
   /// virtual).
   ///
-  /// This returns NULL if the directory doesn't exist.
+  /// This returns a \c std::error_code if there was an error reading the
+  /// directory. If there is no error, the DirectoryEntry is guaranteed to be
+  /// non-NULL.
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  const DirectoryEntry *getDirectory(StringRef DirName,
- bool CacheFailure = true);
+  llvm::ErrorOr
+  getDirectory(StringRef DirName, bool CacheFailure = true);
 
   /// Lookup, cache, and verify the specified file (real or
   /// virtual).
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
+  /// If there is no error, the FileEntry is guaranteed to be non-NULL.
   ///
   /// \param OpenFile if true and the file exists, it will be opened.
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  const FileEntry *getFile(StringRef Filename, bool OpenFile = false,
-   bool CacheFailure = true);
+  llvm::ErrorOr
+  getFile(StringRef Filename, bool OpenFile = false, bool CacheFailure = true);
 
   /// Returns the current file system options
   FileSystemOptions () { return FileSystemOpts; }
@@ -243,8 +251,9 @@ public:
   /// If the path is relative, it will be resolved against the WorkingDir of 
the
   /// FileManager's FileSystemOptions.
   ///
-  /// \returns false on success, true on error.
-  bool getNoncachedStatValue(StringRef Path, llvm::vfs::Status );
+  /// \returns a \c std::error_code describing an error, if there was one
+  std::error_code getNoncachedStatValue(StringRef Path,
+llvm::vfs::Status );
 
   /// Remove the real file \p Entry from the cache.
   void invalidateCache(const FileEntry *Entry);

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=367615=367614=367615=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Aug  1 14:31:49 

[PATCH] D64592: [OpenMP] Fix declare target link implementation

2019-08-01 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367613: [OpenMP] Fix declare target link implementation 
(authored by gbercea, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64592

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/test/OpenMP/declare_target_codegen.cpp
  cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
  cfe/trunk/test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Index: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
===
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp
@@ -20,10 +20,10 @@
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* @bbb to i8*),
 // CHECK-DAG: @ccc = external global i32,
 // CHECK-DAG: @ddd ={{ dso_local | }}global i32 0,
-// CHECK-DAG: @hhh_decl_tgt_ref_ptr = common global i32* null
-// CHECK-DAG: @ggg_decl_tgt_ref_ptr = common global i32* null
-// CHECK-DAG: @fff_decl_tgt_ref_ptr = common global i32* null
-// CHECK-DAG: @eee_decl_tgt_ref_ptr = common global i32* null
+// CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global i32* null
+// CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global i32* null
+// CHECK-DAG: @fff_decl_tgt_ref_ptr = weak global i32* null
+// CHECK-DAG: @eee_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @{{.*}}maini1{{.*}}aaa = internal global i64 23,
 // CHECK-DAG: @b ={{ dso_local | }}global i32 15,
 // CHECK-DAG: @d ={{ dso_local | }}global i32 0,
@@ -32,7 +32,7 @@
 // CHECK-DAG: [[STAT:@.+stat]] = internal global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT_REF:@.+]] = internal constant %struct.S* [[STAT]]
 // CHECK-DAG: @out_decl_target ={{ dso_local | }}global i32 0,
-// CHECK-DAG: @llvm.used = appending global [6 x i8*] [i8* bitcast (void ()* @__omp_offloading__{{.+}}_globals_l[[@LINE+80]]_ctor to i8*), i8* bitcast (void ()* @__omp_offloading__{{.+}}_stat_l[[@LINE+81]]_ctor to i8*),
+// CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* @__omp_offloading__{{.+}}_globals_l[[@LINE+80]]_ctor to i8*), i8* bitcast (void ()* @__omp_offloading__{{.+}}_stat_l[[@LINE+81]]_ctor to i8*)],
 // CHECK-DAG: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (%struct.S** [[STAT_REF]] to i8*)],
 
 // CHECK-DAG: define {{.*}}i32 @{{.*}}{{foo|bar|baz2|baz3|FA|f_method}}{{.*}}()
Index: cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
===
--- cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
+++ cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
@@ -18,24 +18,31 @@
 #define HEADER
 
 // HOST-DAG: @c = external global i32,
-// HOST-DAG: @c_decl_tgt_ref_ptr = global i32* @c
+// HOST-DAG: @c_decl_tgt_ref_ptr = weak global i32* @c
+// HOST-DAG: @[[D:.+]] = internal global i32 2
+// HOST-DAG: @[[D_PTR:.+]] = weak global i32* @[[D]]
 // DEVICE-NOT: @c =
-// DEVICE: @c_decl_tgt_ref_ptr = common global i32* null
-// HOST: [[SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 4]
-// HOST: [[MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 35, i64 531]
+// DEVICE: @c_decl_tgt_ref_ptr = weak global i32* null
+// HOST: [[SIZES:@.+]] = private unnamed_addr constant [3 x i64] [i64 4, i64 4, i64 4]
+// HOST: [[MAPTYPES:@.+]] = private unnamed_addr constant [3 x i64] [i64 35, i64 531, i64 531]
 // HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
 // HOST: @.omp_offloading.entry.c_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name, i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries", align 1
-// DEVICE-NOT: internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
-// CHECK: @llvm.used = appending global [1 x i8*] [i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*)]
+// DEVICE-NOT: internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_{{.*}}_decl_tgt_ref_ptr\00"
+// HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"_{{.*}}d_{{.*}}_decl_tgt_ref_ptr\00"
+// HOST: @.omp_offloading.entry.[[D_PTR]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @[[D_PTR]] to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name{{.*}}, i32 0, i32 0
 
 extern int c;
 #pragma omp declare target link(c)
 
+static int d = 2;
+#pragma omp declare target link(d)
+
 int maini1() {
   int a;
 #pragma omp target map(tofrom : a)
   {
 a = c;
+d++;
   }
 #pragma omp target
 #pragma omp teams
@@ -43,29 +50,38 @@
   return 0;
 }
 
-// DEVICE: 

r367613 - [OpenMP] Fix declare target link implementation

2019-08-01 Thread Gheorghe-Teodor Bercea via cfe-commits
Author: gbercea
Date: Thu Aug  1 14:15:58 2019
New Revision: 367613

URL: http://llvm.org/viewvc/llvm-project?rev=367613=rev
Log:
[OpenMP] Fix declare target link implementation

Summary:
This patch fixes the case where variables in different compilation units or the 
same compilation unit are under the declare target link clause AND have the 
same name.
This also fixes the name clash error that occurs when unified memory is 
activated.
The changes in this patch include:
- Pointers to internal variables are given unique names.
- Externally visible variables are given the same name as before.
- All pointer variables (external or internal) are weakly linked.

Reviewers: ABataev, jdoerfert, caomhin

Reviewed By: ABataev

Subscribers: lebedev.ri, guansong, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=367613=367612=367613=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Aug  1 14:15:58 2019
@@ -2552,6 +2552,32 @@ CGOpenMPRuntime::createDispatchNextFunct
   return CGM.CreateRuntimeFunction(FnTy, Name);
 }
 
+/// Obtain information that uniquely identifies a target entry. This
+/// consists of the file and device IDs as well as line number associated with
+/// the relevant entry source location.
+static void getTargetEntryUniqueInfo(ASTContext , SourceLocation Loc,
+ unsigned , unsigned ,
+ unsigned ) {
+  SourceManager  = C.getSourceManager();
+
+  // The loc should be always valid and have a file ID (the user cannot use
+  // #pragma directives in macros)
+
+  assert(Loc.isValid() && "Source location is expected to be always valid.");
+
+  PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+  assert(PLoc.isValid() && "Source location is expected to be always valid.");
+
+  llvm::sys::fs::UniqueID ID;
+  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+SM.getDiagnostics().Report(diag::err_cannot_open_file)
+<< PLoc.getFilename() << EC.message();
+
+  DeviceID = ID.getDevice();
+  FileID = ID.getFile();
+  LineNum = PLoc.getLine();
+}
+
 Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
   if (CGM.getLangOpts().OpenMPSimd)
 return Address::invalid();
@@ -2563,19 +2589,27 @@ Address CGOpenMPRuntime::getAddrOfDeclar
 SmallString<64> PtrName;
 {
   llvm::raw_svector_ostream OS(PtrName);
-  OS << CGM.getMangledName(GlobalDecl(VD)) << "_decl_tgt_ref_ptr";
+  OS << CGM.getMangledName(GlobalDecl(VD));
+  if (!VD->isExternallyVisible()) {
+unsigned DeviceID, FileID, Line;
+getTargetEntryUniqueInfo(CGM.getContext(),
+ VD->getCanonicalDecl()->getBeginLoc(),
+ DeviceID, FileID, Line);
+OS << llvm::format("_%x", FileID);
+  }
+  OS << "_decl_tgt_ref_ptr";
 }
 llvm::Value *Ptr = CGM.getModule().getNamedValue(PtrName);
 if (!Ptr) {
   QualType PtrTy = CGM.getContext().getPointerType(VD->getType());
   Ptr = 
getOrCreateInternalVariable(CGM.getTypes().ConvertTypeForMem(PtrTy),
 PtrName);
-  if (!CGM.getLangOpts().OpenMPIsDevice) {
-auto *GV = cast(Ptr);
-GV->setLinkage(llvm::GlobalValue::ExternalLinkage);
+
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+
+  if (!CGM.getLangOpts().OpenMPIsDevice)
 GV->setInitializer(CGM.GetAddrOfGlobal(VD));
-  }
-  CGM.addUsedGlobal(cast(Ptr));
   registerTargetGlobalVariable(VD, cast(Ptr));
 }
 return Address(Ptr, CGM.getContext().getDeclAlign(VD));
@@ -2749,32 +2783,6 @@ llvm::Function *CGOpenMPRuntime::emitThr
   return nullptr;
 }
 
-/// Obtain information that uniquely identifies a target entry. This
-/// consists of the file and device IDs as well as line number associated with
-/// the relevant entry source location.
-static void getTargetEntryUniqueInfo(ASTContext , SourceLocation Loc,
- unsigned , unsigned ,
- unsigned ) {
-  SourceManager  = C.getSourceManager();
-
-  // The loc should be always valid and have a file ID (the user cannot use
-  // #pragma directives in macros)
-
-  assert(Loc.isValid() && "Source location is expected to be always valid.");
-
-  PresumedLoc PLoc = SM.getPresumedLoc(Loc);
-  assert(PLoc.isValid() && "Source location is expected to be always 

[PATCH] D65487: [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

2019-08-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

I thereby confirm that i've no idea what was this code about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65487



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


[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

2019-08-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Sry, should have approved this ages ago >.<




Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > NoQ wrote:
> > > > > Huh, so there's not even a note in `getHalfPoint()`, just 
> > > > > calling..returning? This definitely needs some attention from 
> > > > > `NoStoreFuncVisitor`.
> > > > > 
> > > > > Generally, i think this is probably the single place where we do 
> > > > > really want some info about what happens in `getHalfPoint()`. The 
> > > > > report that consists only of "p is initialized..." and "...p is 
> > > > > uninitialized" is pretty weird. Btw, could you write down the full 
> > > > > warning text in this test? How bad this actually is?
> > > > Wild idea: `UninitializedObjectChecker` exposes it's main logic through 
> > > > a header file, how about we use it when we find a read of an 
> > > > uninitialized region?
> > > Passed-by-value struct argument contains uninitialized data (e.g., field: 
> > > 'y')
> > > 
> > > Quite good imo.
> > What specific logic are you talking about? You mean it'd scan the structure 
> > for uninitialized fields and present the results of the scan to the user in 
> > a note?
> >What specific logic are you talking about? You mean it'd scan the structure 
> >for uninitialized fields and present the results of the scan to the user in 
> >a note?
> Nvm, looked at the code, realized that what I said made no sense. What we are 
> really missing here is a `trackRegionValue()` function :^)
> 
> Btw, I wasted s much time on figuring out that you don't get ANY notes 
> whatsoever if you make this a cpp file rather than a c file, only the 
> warning... Is this intended?
> Btw, I wasted s much time on figuring out that you don't get ANY notes 
> whatsoever if you make this a cpp file rather than a c file, only the 
> warning... Is this intended?

At a glance, the behavior of this code in C and C++ is ridiculously different. 
The way structures are returned-by-value is one of the glaring differences 
between C and C++. Even at runtime / ABI / calling convention level: in C it's 
acceptable to simply pass the structure through registers (or put it on the 
stack, wherever the return value normally lives), however in C++ the calling 
convention usually requres you to pass return address as a separate hidden 
parameter so that to use it as "this" in the structure's constructor (and then 
later use it for RVO and NRVO which may span multiple stack frames) . And the 
Static Analyzer also deals with completely different ASTs. So i'm not surprised 
that there's a difference.

But i would also probably not want there to be a difference. If you could turn 
it into a sensible bug report i guess it could be helpful.


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

https://reviews.llvm.org/D64232



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


[PATCH] D65545: Handle some fs::remove failures

2019-08-01 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 212899.
jfb marked 11 inline comments as done.
jfb added a comment.

- Return llvm::Error from ASTUnit::Save
- Update per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Rewrite/Rewriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1gen_reproducer_main.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  llvm/include/llvm/Support/Error.h
  llvm/include/llvm/Support/FileUtilities.h
  llvm/lib/Support/GraphWriter.cpp
  llvm/lib/Support/LockFileManager.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp

Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -25,7 +25,9 @@
 ToolOutputFile::CleanupInstaller::~CleanupInstaller() {
   // Delete the file if the client hasn't told us not to.
   if (!Keep && Filename != "-")
-sys::fs::remove(Filename);
+if (std::error_code EC = sys::fs::remove(Filename))
+  report_fatal_error("Failed removing file \"" + Filename +
+ "\": " + EC.message());
 
   // Ok, the file is successfully written and closed, or deleted. There's no
   // further need to clean it up on signals.
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1182,9 +1182,10 @@
   if (RenameEC) {
 // If we can't rename, try to copy to work around cross-device link issues.
 RenameEC = sys::fs::copy_file(TmpName, Name);
-// If we can't rename or copy, discard the temporary file.
+// If we can't rename or copy, discard the temporary file, ignoring any
+// further failure.
 if (RenameEC)
-  remove(TmpName);
+  (void)remove(TmpName);
   }
   sys::DontRemoveFileOnSignal(TmpName);
 #endif
Index: llvm/lib/Support/LockFileManager.cpp
===
--- llvm/lib/Support/LockFileManager.cpp
+++ llvm/lib/Support/LockFileManager.cpp
@@ -47,14 +47,16 @@
 /// \param LockFileName The name of the lock file to read.
 ///
 /// \returns The process ID of the process that owns this lock file
-Optional >
+Optional>
 LockFileManager::readLockFile(StringRef LockFileName) {
   // Read the owning host and PID out of the lock file. If it appears that the
   // owning process is dead, the lock file is invalid.
   ErrorOr> MBOrErr =
   MemoryBuffer::getFile(LockFileName);
   if (!MBOrErr) {
-sys::fs::remove(LockFileName);
+if (std::error_code EC = sys::fs::remove(LockFileName))
+  report_fatal_error("Unable to remove invalid lock file \"" +
+ LockFileName + "\": " + EC.message());
 return None;
   }
   MemoryBuffer  = *MBOrErr.get();
@@ -71,7 +73,9 @@
   }
 
   // Delete the lock file. It's invalid anyway.
-  sys::fs::remove(LockFileName);
+  if (std::error_code EC = sys::fs::remove(LockFileName))
+report_fatal_error("Unable to remove invalid lock file \"" + LockFileName +
+   "\": " + EC.message());
   return None;
 }
 
@@ -144,7 +148,9 @@
   // released.
   return;
 }
-sys::fs::remove(Filename);
+if (std::error_code EC = sys::fs::remove(Filename))
+  report_fatal_error("Unable to remove unique lock file on signal \"" +
+ Filename + "\": " + EC.message());
 sys::DontRemoveFileOnSignal(Filename);
   }
 
@@ -204,8 +210,9 @@
   // unique lock file, and fail.
   std::string S("failed to write to ");
   S.append(UniqueLockFileName.str());
+  if (std::error_code EC = sys::fs::remove(UniqueLockFileName))
+S.append(", also failed to remove the lockfile");
   setError(Out.error(), S);
-  sys::fs::remove(UniqueLockFileName);
   return;
 }
   }
@@ -234,8 +241,12 @@
 // Someone else managed to create the lock file first. Read the process ID
 // from the lock file.
 if ((Owner = readLockFile(LockFileName))) {
-  // Wipe out our unique lock file (it's useless now)
-  sys::fs::remove(UniqueLockFileName);
+  // Wipe out our unique lock file (it's useless now).
+  if ((EC = sys::fs::remove(UniqueLockFileName))) {
+std::string S("failed to remove useless lockfile ");
+S.append(UniqueLockFileName.str());
+setError(EC, S);
+  }
   return;
 }
 
@@ -283,8 +294,12 @@
 return;
 
   // 

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367608: [analyzer] StackFrameContext: Add 
NodeBuilderContext::blockCount() to its… (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65587?vs=212895=212898#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65587

Files:
  cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  cfe/trunk/test/Analysis/loop-block-counts.c
  cfe/trunk/test/Analysis/loop-unrolling.cpp
  cfe/trunk/test/Analysis/stack-frame-context-revision.cpp

Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
===
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
@@ -183,9 +183,8 @@
   const ImplicitParamDecl *getSelfDecl() const;
 
   const StackFrameContext *getStackFrame(LocationContext const *Parent,
- const Stmt *S,
- const CFGBlock *Blk,
- unsigned Idx);
+ const Stmt *S, const CFGBlock *Blk,
+ unsigned BlockCount, unsigned Idx);
 
   const BlockInvocationContext *
   getBlockInvocationContext(const LocationContext *parent,
@@ -303,15 +302,19 @@
   // The parent block of the callsite.
   const CFGBlock *Block;
 
+  // The number of times the 'Block' has been visited.
+  // It allows discriminating between stack frames of the same call that is
+  // called multiple times in a loop.
+  const unsigned BlockCount;
+
   // The index of the callsite in the CFGBlock.
-  unsigned Index;
+  const unsigned Index;
 
   StackFrameContext(AnalysisDeclContext *ctx, const LocationContext *parent,
-const Stmt *s, const CFGBlock *blk,
-unsigned idx,
-int64_t ID)
-  : LocationContext(StackFrame, ctx, parent, ID), CallSite(s),
-Block(blk), Index(idx) {}
+const Stmt *s, const CFGBlock *blk, unsigned blockCount,
+unsigned idx, int64_t ID)
+  : LocationContext(StackFrame, ctx, parent, ID), CallSite(s), Block(blk),
+BlockCount(blockCount), Index(idx) {}
 
 public:
   ~StackFrameContext() override = default;
@@ -329,9 +332,10 @@
 
   static void Profile(llvm::FoldingSetNodeID , AnalysisDeclContext *ctx,
   const LocationContext *parent, const Stmt *s,
-  const CFGBlock *blk, unsigned idx) {
+  const CFGBlock *blk, unsigned blockCount, unsigned idx) {
 ProfileCommon(ID, StackFrame, ctx, parent, s);
 ID.AddPointer(blk);
+ID.AddInteger(blockCount);
 ID.AddInteger(idx);
   }
 
@@ -410,8 +414,8 @@
 
   const StackFrameContext *getStackFrame(AnalysisDeclContext *ctx,
  const LocationContext *parent,
- const Stmt *s,
- const CFGBlock *blk, unsigned idx);
+ const Stmt *s, const CFGBlock *blk,
+ unsigned blockCount, unsigned idx);
 
   const ScopeContext *getScope(AnalysisDeclContext *ctx,
const LocationContext *parent,
@@ -483,26 +487,25 @@
   bool synthesizeBodies() const { return SynthesizeBodies; }
 
   const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx,
- LocationContext const *Parent,
- const Stmt *S,
- const CFGBlock *Blk,
- unsigned Idx) {
-return LocContexts.getStackFrame(Ctx, Parent, S, Blk, Idx);
+ const LocationContext *Parent,
+ const Stmt *S, const CFGBlock *Blk,
+ unsigned BlockCount, unsigned Idx) {
+return LocContexts.getStackFrame(Ctx, Parent, S, Blk, BlockCount, Idx);
   }
 
   // Get the top level stack frame.
   const StackFrameContext *getStackFrame(const Decl *D) {
 return LocContexts.getStackFrame(getContext(D), nullptr, nullptr, nullptr,
- 0);
+ 0, 0);
   }
 
   // Get a stack frame with parent.
   StackFrameContext const *getStackFrame(const Decl *D,
-   

r367608 - [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via cfe-commits
Author: charusso
Date: Thu Aug  1 13:41:13 2019
New Revision: 367608

URL: http://llvm.org/viewvc/llvm-project?rev=367608=rev
Log:
[analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its 
profile

Summary:
It allows discriminating between stack frames of the same call that is
called multiple times in a loop.

Thanks to Artem Dergachev for the great idea!

Reviewed By: NoQ

Tags: #clang

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

Added:
cfe/trunk/test/Analysis/stack-frame-context-revision.cpp
Modified:
cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/loop-block-counts.c
cfe/trunk/test/Analysis/loop-unrolling.cpp

Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=367608=367607=367608=diff
==
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Thu Aug  1 13:41:13 
2019
@@ -183,9 +183,8 @@ public:
   const ImplicitParamDecl *getSelfDecl() const;
 
   const StackFrameContext *getStackFrame(LocationContext const *Parent,
- const Stmt *S,
- const CFGBlock *Blk,
- unsigned Idx);
+ const Stmt *S, const CFGBlock *Blk,
+ unsigned BlockCount, unsigned Idx);
 
   const BlockInvocationContext *
   getBlockInvocationContext(const LocationContext *parent,
@@ -303,15 +302,19 @@ class StackFrameContext : public Locatio
   // The parent block of the callsite.
   const CFGBlock *Block;
 
+  // The number of times the 'Block' has been visited.
+  // It allows discriminating between stack frames of the same call that is
+  // called multiple times in a loop.
+  const unsigned BlockCount;
+
   // The index of the callsite in the CFGBlock.
-  unsigned Index;
+  const unsigned Index;
 
   StackFrameContext(AnalysisDeclContext *ctx, const LocationContext *parent,
-const Stmt *s, const CFGBlock *blk,
-unsigned idx,
-int64_t ID)
-  : LocationContext(StackFrame, ctx, parent, ID), CallSite(s),
-Block(blk), Index(idx) {}
+const Stmt *s, const CFGBlock *blk, unsigned blockCount,
+unsigned idx, int64_t ID)
+  : LocationContext(StackFrame, ctx, parent, ID), CallSite(s), Block(blk),
+BlockCount(blockCount), Index(idx) {}
 
 public:
   ~StackFrameContext() override = default;
@@ -329,9 +332,10 @@ public:
 
   static void Profile(llvm::FoldingSetNodeID , AnalysisDeclContext *ctx,
   const LocationContext *parent, const Stmt *s,
-  const CFGBlock *blk, unsigned idx) {
+  const CFGBlock *blk, unsigned blockCount, unsigned idx) {
 ProfileCommon(ID, StackFrame, ctx, parent, s);
 ID.AddPointer(blk);
+ID.AddInteger(blockCount);
 ID.AddInteger(idx);
   }
 
@@ -410,8 +414,8 @@ public:
 
   const StackFrameContext *getStackFrame(AnalysisDeclContext *ctx,
  const LocationContext *parent,
- const Stmt *s,
- const CFGBlock *blk, unsigned idx);
+ const Stmt *s, const CFGBlock *blk,
+ unsigned blockCount, unsigned idx);
 
   const ScopeContext *getScope(AnalysisDeclContext *ctx,
const LocationContext *parent,
@@ -483,26 +487,25 @@ public:
   bool synthesizeBodies() const { return SynthesizeBodies; }
 
   const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx,
- LocationContext const *Parent,
- const Stmt *S,
- const CFGBlock *Blk,
- unsigned Idx) {
-return LocContexts.getStackFrame(Ctx, Parent, S, Blk, Idx);
+ const LocationContext *Parent,
+ const Stmt *S, const CFGBlock *Blk,
+ unsigned BlockCount, unsigned Idx) {
+return LocContexts.getStackFrame(Ctx, Parent, S, Blk, BlockCount, Idx);
   }
 
   // Get the top level stack frame.
   const StackFrameContext *getStackFrame(const Decl *D) {
 return 

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

2019-08-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Fantastic! Let's open the wording bikeshed season?

I suspect that a simple "(The) Value -> Condition value" change would have 
worked better.

Another variant: "Value ..., which participates in a condition later".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65575



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


[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

Thanks for the review and for the idea!


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

https://reviews.llvm.org/D65587



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


[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a subscriber: george.karpenkov.
Charusso added inline comments.



Comment at: clang/test/Analysis/loop-unrolling.cpp:349-353
 #ifdef DFS
-clang_analyzer_numTimesReached(); // expected-warning {{10}}
+clang_analyzer_numTimesReached(); // expected-warning {{16}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+clang_analyzer_numTimesReached(); // expected-warning {{8}}
 #endif

NoQ wrote:
> I'm mildly curious what happened here.
I will leave that comment not 'Done' as some kind of `FIXME: Measure the loop 
unrolling.`
But I believe these numbers supposed to be here from the start when 
@george.karpenkov made the traversing coverage-based (source: 
https://youtu.be/4n3l-ZcDJNY?t=540).


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

https://reviews.llvm.org/D65587



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


[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 212895.
Charusso marked an inline comment as done.
Charusso edited the summary of this revision.
Charusso added a comment.

- Fix.


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

https://reviews.llvm.org/D65587

Files:
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/loop-block-counts.c
  clang/test/Analysis/loop-unrolling.cpp
  clang/test/Analysis/stack-frame-context-revision.cpp

Index: clang/test/Analysis/stack-frame-context-revision.cpp
===
--- /dev/null
+++ clang/test/Analysis/stack-frame-context-revision.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -verify %s
+
+// expected-no-diagnostics:
+// From now the profile of the 'StackFrameContext' also contains the
+// 'NodeBuilderContext::blockCount()'. With this addition we can distinguish
+// between the 'StackArgumentsSpaceRegion' of the 'P' arguments being different
+// on every iteration.
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+template 
+struct SmarterPointer {
+  PointerTy getFromVoidPointer(void *P) const {
+return static_cast(P);
+  }
+
+  PointerTy getPointer() const {
+return getFromVoidPointer(reinterpret_cast(Value));
+  }
+
+  intptr_t Value = 13;
+};
+
+struct Node {
+  SmarterPointer Pred;
+};
+
+void test(Node *N) {
+  while (N) {
+SmarterPointer Next = N->Pred;
+delete N;
+
+N = Next.getPointer();
+// no-warning: 'Use of memory after it is freed' was here as the same
+// 'StackArgumentsSpaceRegion' purged out twice as 'P'.
+  }
+}
Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -347,9 +347,9 @@
 int simple_unknown_bound_loop() {
   for (int i = 2; i < getNum(); i++) {
 #ifdef DFS
-clang_analyzer_numTimesReached(); // expected-warning {{10}}
+clang_analyzer_numTimesReached(); // expected-warning {{16}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+clang_analyzer_numTimesReached(); // expected-warning {{8}}
 #endif
   }
   return 0;
@@ -368,10 +368,10 @@
 int nested_inlined_no_unroll1() {
   int k;
   for (int i = 0; i < 9; i++) {
-#ifdef ANALYZER_CM_Z3
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+#ifdef DFS
+clang_analyzer_numTimesReached(); // expected-warning {{18}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{15}}
+clang_analyzer_numTimesReached(); // expected-warning {{14}}
 #endif
 k = simple_unknown_bound_loop();  // reevaluation without inlining, splits the state as well
   }
Index: clang/test/Analysis/loop-block-counts.c
===
--- clang/test/Analysis/loop-block-counts.c
+++ clang/test/Analysis/loop-block-counts.c
@@ -12,7 +12,7 @@
   for (int i = 0; i < 2; ++i)
 callee([i]);
   // FIXME: Should be UNKNOWN.
-  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{FALSE}}
 }
 
 void loopWithCall() {
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -451,9 +451,8 @@
   // Construct a new stack frame for the callee.
   AnalysisDeclContext *CalleeADC = AMgr.getAnalysisDeclContext(D);
   const StackFrameContext *CalleeSFC =
-CalleeADC->getStackFrame(ParentOfCallee, CallE,
- currBldrCtx->getBlock(),
- currStmtIdx);
+  CalleeADC->getStackFrame(ParentOfCallee, CallE, currBldrCtx->getBlock(),
+   currBldrCtx->blockCount(), currStmtIdx);
 
   CallEnter Loc(CallE, CalleeSFC, CurLC);
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -323,7 +323,8 @@
   CallEventManager  = getStateManager().getCallEventManager();
   SVal V = UnknownVal();
   auto getArgLoc = [&](CallEventRef<> Caller) -> Optional {
-const LocationContext *FutureSFC = Caller->getCalleeStackFrame();
+const LocationContext *FutureSFC =
+Caller->getCalleeStackFrame(currBldrCtx->blockCount());
 // Return early if we are unable to reliably foresee
 // the future stack 

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

2019-08-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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


[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

2019-08-01 Thread Charles Zhang via Phabricator via cfe-commits
czhang updated this revision to Diff 212890.
czhang marked an inline comment as done.
czhang added a comment.

Add example in documentation.


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

https://reviews.llvm.org/D62829

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp

Index: clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s bugprone-dynamic-static-initializers %t
+
+int fact(int n) {
+  return (n == 0) ? 1 : n * fact(n - 1);
+}
+
+int static_thing = fact(5);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'static_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+
+int sample() {
+int x;
+return x;
+}
+
+int dynamic_thing = sample();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: static variable 'dynamic_thing' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+
+int not_so_bad = 12 + 4942; // no warning
+
+extern int bar();
+
+int foo() {
+  static int k = bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'k' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+  return k;
+}
+
+int bar2() {
+  return 7;
+}
+
+int foo2() {
+  // This may work fine when optimization is enabled because bar() can
+  // be turned into a constant 7.  But without optimization, it can
+  // cause problems. Therefore, we must err on the side of conservatism.
+  static int x = bar2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static variable 'x' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
+  return x;
+}
+
+int foo3() {
+  static int p = 7 + 83; // no warning
+  return p;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -43,6 +43,7 @@
bugprone-branch-clone
bugprone-copy-constructor-init
bugprone-dangling-handle
+   bugprone-dynamic-static-initializers
bugprone-exception-escape
bugprone-fold-init-type
bugprone-forward-declaration-namespace
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - bugprone-dynamic-static-initializers
+
+bugprone-dynamic-static-initializers
+
+
+Finds instances of static variables that are dynamically initialized
+in header files.
+
+This can pose problems in certain multithreaded contexts. For example,
+when disabling compiler generated synchronization instructions for
+static variables initialized at runtime, even if a particular project
+takes the necessary precautions to prevent race conditions during
+initialization, header files included from other projects may
+not. Therefore, such a check is helpful for ensuring that disabling
+synchronization for static variable initialization will not cause
+problems.
+
+Consider the following code:
+
+-- code-block:: c
+
+int foo() {
+  static int k = bar();
+  return k;
+}
+
+When synchronization of static initialization is disabled, if two threads both call `foo` for the first time, there is the possibility that `k` will be double initialized, creating a race condition.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,12 @@
 
   This checks ensures that ``pipe2()`` is called with the O_CLOEXEC flag.
 
+- New :doc:`bugprone-dynamic-static-initializers
+  ` check.
+
+  Finds instances where variables with static storage are initialized
+  dynamically in header files.
+
 - New :doc:`bugprone-unhandled-self-assignment
   ` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
@@ -0,0 +1,43 

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

2019-08-01 Thread Nathan Huckleberry via Phabricator via cfe-commits
Nathan-Huckleberry updated this revision to Diff 212889.
Nathan-Huckleberry added a comment.

- Support python2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454

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

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

2019-08-01 Thread Charles Zhang via Phabricator via cfe-commits
czhang marked 3 inline comments as done.
czhang added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-dynamic-static-initializers.hpp:33-35
+  // This may work fine when optimization is enabled because bar() can
+  // be turned into a constant 7.  But without optimization, it can
+  // cause problems. Therefore, we must err on the side of conservatism.

aaron.ballman wrote:
> What problems can be caused here? Typically, dynamic init is only problematic 
> when it happens before main() is executed (because of initialization order 
> dependencies), but that doesn't apply to local statics.
Consider the case when synchronization is disabled for static initialization, 
and two threads call `foo2` for the first time. It may be the case that they 
both try and initialize the static variable at the same time with different 
values (since the dynamic initializer may not be pure), creating a race 
condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62829



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


[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Jan Korous via Phabricator via cfe-commits
jkorous accepted this revision.
jkorous marked an inline comment as done.
jkorous added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks for all the work here!




Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156
+  auto newE = FileMgr->getFile(tempPath);
+  if (newE) {
+remap(origFE, *newE);

harlanhaskins wrote:
> jkorous wrote:
> > It seems like we're now avoiding the assert in `remap()` we'd hit 
> > previously. Is this fine?
> If we want to trap if this temp file fails, then I'm happy removing the 
> condition check. Do you think this should trap on failure or should it check 
> the condition?
To be clear - I didn't have any opinion, just noticed there's a functional 
change and pointed that out.
On the other hand I assume your solution is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


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

2019-08-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



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

emmettneyman wrote:
> aaron.ballman wrote:
> > Why does this have a `Clang` spelling in addition to the `GCC` spelling? I 
> > think it only needs the `GCC` spelling.
> I kept the `Clang` spelling so that there's naming consistency between the 
> two attributes (`requires_designator` and `requires_init`). I can remove it 
> if it's redundant/unnecessary, let me know.
This is tricky. I don't see any value in `[[clang::requires_designator]]` in 
C++ mode because `[[gnu::designated_init]]` suffices, so I'd kind of prefer to 
see the `Clang` spelling go away. This attribute can also be used in C with 
`__attribute__((designated_init))` which is great, but I'd also like to see it 
be made available in C with double square bracket syntax (supported in C2x or 
via a feature flag). However, there is no `[[gnu::designated_init]]` supported 
by GCC, so it would be an abuse of the `gnu` namespace to add it there.  Having 
a `Clang` spelling gets around that issue. That said, I suspect GCC will 
support C2x attributes at some point, so I would expect this attribute to be 
supported there eventually, so the issue remains.

I think I've convinced myself that we should just drop the `Clang` spelling and 
wait to handle the C2x square bracket attribute until later, since C users can 
still use the `GNU` spelling. You should also rename the attribute 
`DesignatedInit` and change the other places with the older spelling.



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

emmettneyman wrote:
> aaron.ballman wrote:
> > Why is it valuable to differ from GCC's behavior in this regard?
> The motivation behind GCC's attribute seems to be to avoid using positional 
> arguments:
> > The intent of this attribute is to allow the programmer to indicate that a 
> > structure’s layout may change, and that therefore relying on positional 
> > initialization will result in future breakage.
> 
> The motivation behind this attribute is a little bit different. Instead of 
> avoiding the use of positional arguments, the goal of this attribute is to 
> bring ObjC-style named parameters to C/C++ and to be able to enforce their 
> use in certain situations. In line with this goal, we wanted to forbid the 
> following pattern:
> ```
> Foo foo;
> foo.x = 42;
> foo.y = 36;
> ```
> That's why this attribute enforces that all declarations be brace-initialized.
I may still be missing something, but your rationale feels a bit more like a 
style-based choice than preventing common mistakes that are hard to track down; 
I find the GCC rationale to be more compelling. What do you think about 
matching the GCC behavior in the compiler warning, but adding the additional 
restrictions you'd like to see as a clang-tidy check?



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

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

S s;
```
This would fail because `s` is not initialized with curly braces? I don't think 
that's a particularly good behavior for C++.



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

emmettneyman wrote:
> aaron.ballman wrote:
> > This attribute spelling seems wrong, it should be `designated_init`, no?
> 

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Ah, omission of the century :/

Thanks, this looks immediately good!




Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:305
 
+  // The 'NodeBuilderContext::blockCount()'.
+  const unsigned BlockCount;

"The number of times the Block has been visited. Allows discriminating between 
stack frames of the same call that is called multiple times in a loop."



Comment at: clang/test/Analysis/loop-unrolling.cpp:349-353
 #ifdef DFS
-clang_analyzer_numTimesReached(); // expected-warning {{10}}
+clang_analyzer_numTimesReached(); // expected-warning {{16}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+clang_analyzer_numTimesReached(); // expected-warning {{8}}
 #endif

I'm mildly curious what happened here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65587



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


[PATCH] D65573: Add User docs for ASTImporter

2019-08-01 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

Thank you for writing this up! I just have a few minor comments.




Comment at: clang/docs/LibASTImporter.rst:110
+
+Now we create the Importer and do the import:
+

Maybe helpful to link to the [Matching the Clang 
AST](https://clang.llvm.org/docs/LibASTMatchers.html) and [AST Matcher 
Reference](https://clang.llvm.org/docs/LibASTMatchersReference.html)



Comment at: clang/docs/LibASTImporter.rst:283
+However, there may be cases when both of the contexts have a definition for a 
given symbol.
+If these definitions differ then we have a name conflict, otherwise known as 
ODR (one definition rule) violation.
+Let's modify the previous tool we had written and try to import a 
``ClassTemplateSpecializationDecl`` with a conflicting definition:

Note ODR is a C++ concept C does not have ODR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65573



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


[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

2019-08-01 Thread Anton Bikineev via Phabricator via cfe-commits
AntonBikineev updated this revision to Diff 212882.
AntonBikineev added a comment.

Roman, -ast-dump shows the correct result, because the correct address space is 
encoded in qualifiers and is passed to Context.getAddrSpaceQualType(...) later 
in this function. This is why this bug is not observable from the AST. But if 
you try accessing real Attr* from TypeSourceInfo, you'll see this mismatch.


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

https://reviews.llvm.org/D65589

Files:
  clang/lib/Sema/SemaType.cpp
  clang/unittests/AST/ASTTraverserTest.cpp


Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.


Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-08-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

This seems reasonable to me except for the fact that the script doesn't run 
when I try it locally.

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

I don't think that API is available in Python 2.7, which is our current min 
required version of Python. That's the last outstanding item for the review 
that I can see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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


[PATCH] D64564: Loop pragma parsing. NFC.

2019-08-01 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 212878.
SjoerdMeijer added a comment.

Fixed the string problems.


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

https://reviews.llvm.org/D64564

Files:
  clang/lib/Parse/ParsePragma.cpp


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,15 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  StringRef ClangLoopStr = "clang loop " + Str.str();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", ClangLoopStr)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1038,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,15 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  StringRef ClangLoopStr = "clang loop " + Str.str();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", ClangLoopStr)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1038,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65543: Use library basenames when autolinking on Windows

2019-08-01 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

I kind of like the current behavior since it's consistent with how resource 
dirs are passed to cc1, and how __FILE__ expands if you give a relative work to 
clang, and how debug info paths look if you pass relative paths to clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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


[PATCH] D65597: WIP: Builtins: Start adding half versions of math builtins

2019-08-01 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision.
arsenm added reviewers: rsmith, rjmccall, Anastasia, yaxunl.
Herald added a subscriber: wdng.

The implementation of the OpenCL builtin currently library uses 2 different 
hacks to get to the corresponding IR intrinsics from the source. This will 
allow removal of those.

I'm not sure this is the right naming scheme, but tries to follow the
single character suffix like f/l. This is a problem for sin/cos, since
__builtin_sinh already means double sinh etc. Another alternative might be _f16


https://reviews.llvm.org/D65597

Files:
  include/clang/Basic/Builtins.def


Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -181,6 +181,7 @@
 BUILTIN(__builtin_ceil , "dd"  , "Fnc")
 BUILTIN(__builtin_ceilf, "ff"  , "Fnc")
 BUILTIN(__builtin_ceill, "LdLd", "Fnc")
+BUILTIN(__builtin_ceilh, "hh"  , "Fnc")
 BUILTIN(__builtin_cos , "dd"  , "Fne")
 BUILTIN(__builtin_cosf, "ff"  , "Fne")
 BUILTIN(__builtin_cosh , "dd"  , "Fne")
@@ -208,15 +209,19 @@
 BUILTIN(__builtin_floor , "dd"  , "Fnc")
 BUILTIN(__builtin_floorf, "ff"  , "Fnc")
 BUILTIN(__builtin_floorl, "LdLd", "Fnc")
+BUILTIN(__builtin_floorh, "hh"  , "Fnc")
 BUILTIN(__builtin_fma, "", "Fne")
 BUILTIN(__builtin_fmaf, "", "Fne")
 BUILTIN(__builtin_fmal, "LdLdLdLd", "Fne")
+BUILTIN(__builtin_fmah, "", "Fne")
 BUILTIN(__builtin_fmax, "ddd", "Fnc")
 BUILTIN(__builtin_fmaxf, "fff", "Fnc")
 BUILTIN(__builtin_fmaxl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_fmaxf, "hhh", "Fnc")
 BUILTIN(__builtin_fmin, "ddd", "Fnc")
 BUILTIN(__builtin_fminf, "fff", "Fnc")
 BUILTIN(__builtin_fminl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_fminh, "hhh", "Fnc")
 BUILTIN(__builtin_hypot , "ddd"  , "Fne")
 BUILTIN(__builtin_hypotf, "fff"  , "Fne")
 BUILTIN(__builtin_hypotl, "LdLdLd", "Fne")
@@ -271,6 +276,7 @@
 BUILTIN(__builtin_rint , "dd", "Fnc")
 BUILTIN(__builtin_rintf, "ff", "Fnc")
 BUILTIN(__builtin_rintl, "LdLd", "Fnc")
+BUILTIN(__builtin_rinth, "hh", "Fnc")
 BUILTIN(__builtin_round, "dd"  , "Fnc")
 BUILTIN(__builtin_roundf, "ff"  , "Fnc")
 BUILTIN(__builtin_roundl, "LdLd"  , "Fnc")
@@ -289,6 +295,7 @@
 BUILTIN(__builtin_sqrt , "dd"  , "Fne")
 BUILTIN(__builtin_sqrtf, "ff"  , "Fne")
 BUILTIN(__builtin_sqrtl, "LdLd", "Fne")
+BUILTIN(__builtin_sqrth, "ff"  , "Fne")
 BUILTIN(__builtin_tan , "dd"  , "Fne")
 BUILTIN(__builtin_tanf, "ff"  , "Fne")
 BUILTIN(__builtin_tanh , "dd"  , "Fne")
@@ -301,6 +308,7 @@
 BUILTIN(__builtin_trunc , "dd", "Fnc")
 BUILTIN(__builtin_truncf, "ff", "Fnc")
 BUILTIN(__builtin_truncl, "LdLd", "Fnc")
+BUILTIN(__builtin_trunch, "hh", "Fnc")
 
 // C99 complex builtins
 BUILTIN(__builtin_cabs, "dXd", "Fne")
@@ -395,6 +403,7 @@
 BUILTIN(__builtin_canonicalize, "dd", "nc")
 BUILTIN(__builtin_canonicalizef, "ff", "nc")
 BUILTIN(__builtin_canonicalizel, "LdLd", "nc")
+BUILTIN(__builtin_canonicalizeh, "hh", "nc")
 
 // Builtins for arithmetic.
 BUILTIN(__builtin_clzs , "iUs"  , "nc")


Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -181,6 +181,7 @@
 BUILTIN(__builtin_ceil , "dd"  , "Fnc")
 BUILTIN(__builtin_ceilf, "ff"  , "Fnc")
 BUILTIN(__builtin_ceill, "LdLd", "Fnc")
+BUILTIN(__builtin_ceilh, "hh"  , "Fnc")
 BUILTIN(__builtin_cos , "dd"  , "Fne")
 BUILTIN(__builtin_cosf, "ff"  , "Fne")
 BUILTIN(__builtin_cosh , "dd"  , "Fne")
@@ -208,15 +209,19 @@
 BUILTIN(__builtin_floor , "dd"  , "Fnc")
 BUILTIN(__builtin_floorf, "ff"  , "Fnc")
 BUILTIN(__builtin_floorl, "LdLd", "Fnc")
+BUILTIN(__builtin_floorh, "hh"  , "Fnc")
 BUILTIN(__builtin_fma, "", "Fne")
 BUILTIN(__builtin_fmaf, "", "Fne")
 BUILTIN(__builtin_fmal, "LdLdLdLd", "Fne")
+BUILTIN(__builtin_fmah, "", "Fne")
 BUILTIN(__builtin_fmax, "ddd", "Fnc")
 BUILTIN(__builtin_fmaxf, "fff", "Fnc")
 BUILTIN(__builtin_fmaxl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_fmaxf, "hhh", "Fnc")
 BUILTIN(__builtin_fmin, "ddd", "Fnc")
 BUILTIN(__builtin_fminf, "fff", "Fnc")
 BUILTIN(__builtin_fminl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_fminh, "hhh", "Fnc")
 BUILTIN(__builtin_hypot , "ddd"  , "Fne")
 BUILTIN(__builtin_hypotf, "fff"  , "Fne")
 BUILTIN(__builtin_hypotl, "LdLdLd", "Fne")
@@ -271,6 +276,7 @@
 BUILTIN(__builtin_rint , "dd", "Fnc")
 BUILTIN(__builtin_rintf, "ff", "Fnc")
 BUILTIN(__builtin_rintl, "LdLd", "Fnc")
+BUILTIN(__builtin_rinth, "hh", "Fnc")
 BUILTIN(__builtin_round, "dd"  , "Fnc")
 BUILTIN(__builtin_roundf, "ff"  , "Fnc")
 BUILTIN(__builtin_roundl, "LdLd"  , "Fnc")
@@ -289,6 +295,7 @@
 BUILTIN(__builtin_sqrt , "dd"  , "Fne")
 BUILTIN(__builtin_sqrtf, "ff"  , "Fne")
 BUILTIN(__builtin_sqrtl, "LdLd", "Fne")
+BUILTIN(__builtin_sqrth, "ff"  , "Fne")
 BUILTIN(__builtin_tan , "dd"  , "Fne")
 BUILTIN(__builtin_tanf, "ff"  , "Fne")
 BUILTIN(__builtin_tanh , "dd"  , "Fne")
@@ -301,6 +308,7 @@
 BUILTIN(__builtin_trunc , "dd", "Fnc")
 

r367602 - Test linux only for absolute paths in the -fuse-ld option

2019-08-01 Thread Yuanfang Chen via cfe-commits
Author: yuanfang
Date: Thu Aug  1 11:49:59 2019
New Revision: 367602

URL: http://llvm.org/viewvc/llvm-project?rev=367602=rev
Log:
Test linux only for absolute paths in the -fuse-ld option

Some target do not use this option and may emit a error message for
using it.

Modified:
cfe/trunk/test/Driver/fuse-ld.c

Modified: cfe/trunk/test/Driver/fuse-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuse-ld.c?rev=367602=367601=367602=diff
==
--- cfe/trunk/test/Driver/fuse-ld.c (original)
+++ cfe/trunk/test/Driver/fuse-ld.c Thu Aug  1 11:49:59 2019
@@ -1,5 +1,6 @@
 // RUN: %clang %s -### \
 // RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
+// RUN: -target x86_64-unknown-linux \
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 


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


[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-08-01 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

Hi Puyan,

I failed to reproduce with llvm.org/master 
(5faa533e47b0e54b04166b0257c5ebb48e6ffcaa 
) on 
Ubuntu 18.04.1 LTS both in debug and release build.

Since it sounds like you can reproduce "reliably" - can you please share more 
info how to reproduce?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58418



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


[PATCH] D65562: Move LangStandard*, InputKind::Language to Basic

2019-08-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Thanks!




Comment at: include/clang/Basic/LangStandard.h:19
+/// standard and possible actions.
+enum Language {
+  Unknown,

Is it feasible to make this an `enum class`? I'm worried about namespace 
clashes on these otherwise very short names, like C, CXX, HIP, etc. It should 
be straightforward and mechanical to replace most existing instances of 
`InputKind::` with `Language::`. It would also remove the need to make an 
exception for `LF_OpenCL`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65562



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


[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins updated this revision to Diff 212860.
harlanhaskins marked 8 inline comments as done.
harlanhaskins added a comment.

Updated in response to feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
  clang-tools-extra/clang-move/Move.cpp
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/FileManager.h
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/ModuleManager.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Refactoring.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  clang/tools/clang-rename/ClangRename.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/RewriterTestContext.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
Index: clang/unittests/Tooling/RewriterTestContext.h
===
--- clang/unittests/Tooling/RewriterTestContext.h
+++ clang/unittests/Tooling/RewriterTestContext.h
@@ -56,9 +56,9 @@
 llvm::MemoryBuffer::getMemBuffer(Content);
 InMemoryFileSystem->addFile(Name, 0, std::move(Source));
 
-const FileEntry *Entry = Files.getFile(Name);
-assert(Entry != nullptr);
-return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
+auto Entry = Files.getFile(Name);
+assert(Entry);
+return Sources.createFileID(*Entry, SourceLocation(), SrcMgr::C_User);
   }
 
   // FIXME: this code is mostly a duplicate of
@@ -73,14 +73,14 @@
  

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins marked 6 inline comments as done and an inline comment as not 
done.
harlanhaskins added inline comments.



Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156
+  auto newE = FileMgr->getFile(tempPath);
+  if (newE) {
+remap(origFE, *newE);

jkorous wrote:
> It seems like we're now avoiding the assert in `remap()` we'd hit previously. 
> Is this fine?
If we want to trap if this temp file fails, then I'm happy removing the 
condition check. Do you think this should trap on failure or should it check 
the condition?



Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:229
 const FileEntry *FileRemapper::getOriginalFile(StringRef filePath) {
-  const FileEntry *file = FileMgr->getFile(filePath);
+  const FileEntry *file;
+  if (auto fileOrErr = FileMgr->getFile(filePath))

jkorous wrote:
> Shouldn't we initialize this guy to `nullptr`?
 good catch!



Comment at: clang/lib/Frontend/FrontendAction.cpp:715
   SmallString<128> DirNative;
-  llvm::sys::path::native(PCHDir->getName(), DirNative);
+  if (*PCHDir == nullptr) {
+llvm::dbgs() << "Directory " << PCHInclude << " was null but no error 
thrown";

jkorous wrote:
> Could this actually happen? I was expecting the behavior to be consistent 
> with `getFile()`.
Oops, debugging code that needs to be removed. Thanks for catching it!



Comment at: clang/lib/Lex/HeaderSearch.cpp:1448
   if (getHeaderSearchOpts().ModuleMapFileHomeIsCwd)
-Dir = FileMgr.getDirectory(".");
+Dir = *FileMgr.getDirectory(".");
   else {

jkorous wrote:
> Seems like we're introducing assert here. Is that fine?
I'll make this conditional, thanks!



Comment at: clang/unittests/Basic/FileManagerTest.cpp:260
+
+  EXPECT_EQ(f1 ? *f1 : nullptr,
+f2 ? *f2 : nullptr);

jkorous wrote:
> Just an idea - should we compare error codes?
Went ahead and added some tests ensuring we get useful error codes for a couple 
of common issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


[PATCH] D65582: IR: accept and print numbered %N names for function args

2019-08-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

This is awesome! Thanks for doing the test updates. As I read it, LLVM will 
still read old-style IR just fine, which seems nice.

I think it would be worth noting this in the 10.0 release notes, since it will 
affect someone upgrading a frontend as you mention.

Otherwise, I think this is basically good to go, unless I'm forgetting 
anything...

celebration_balloons 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65582



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


[PATCH] D65543: Use library basenames when autolinking on Windows

2019-08-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I'll look into that. I also noticed that check-ubsan fails. I think we should 
also change clang's driver to add this libpath when it invokes the linker, so 
that this works transparently when using the GCC-style driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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


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

2019-08-01 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea marked an inline comment as done.
gtbercea added inline comments.



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

Hahnfeld wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > Why?
> > If the pointee has been mapped as TO/FROM/etc already no need to map it 
> > TO/FROM/etc again.
> I'm not sure if there's a test for this. I can't verify right now, so I trust 
> you that it's covered.
I added the test as per Alexey's previous request.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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


[PATCH] D65579: Don't try emitting dllexported explicitly defaulted non-trivial ctors twice during explicit template instantiation definition (PR42857)

2019-08-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

yeesh.


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

https://reviews.llvm.org/D65579



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


[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

2019-08-01 Thread Manikishan Ghantasala via Phabricator via cfe-commits
Manikishan added a comment.

In D64695#1606256 , @rdwampler wrote:

> In D64695#1605676 , @Manikishan 
> wrote:
>
> > In D64695#1590948 , @Manikishan 
> > wrote:
> >
> > > In D64695#1589818 , @lebedev.ri 
> > > wrote:
> > >
> > > > In D64695#1589740 , 
> > > > @Manikishan wrote:
> > > >
> > > > > In D64695#1589508 , 
> > > > > @lebedev.ri wrote:
> > > > >
> > > > > > Is there sufficient test coverage as to what happens if 
> > > > > > `SortPriority` is not set?
> > > > >
> > > > >
> > > > > If SortPriority is not set, the Includes will be grouped without 
> > > > > sorting,
> > > >
> > > >
> > > > Let me rephrase - for the exiting `.clang-format`s, that don't 
> > > > currently specify `SortPriority`,
> > > >  this introduction of `SortPriority` should not change the header 
> > > > handling.
> > > >  Is that the case, and if so is there sufficient test coverage for that?
> > >
> > >
> > > I got your idea now.
> > >  No, there is no test coverage for that case, and with the current patch 
> > > they have to add SortPriority.
> > >  To avoid this shall I set SortPriority as Priority as default if it is 
> > > not defined? I think that will fix the issue.
> >
> >
> > any reviews on it ?
>
>
> That's sounds like it will work. Can you add some additional test cases 
> around this in `SortIncludesTest.cpp`. Also, adding a test case specifically 
> for sorting the NetBSD headers would be good.


Sorry for the delay, I am facing issues with  "NoCrash_Bug34236" will update 
the patch once I am able to fix it.


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] D65592: [Parser] Avoid spurious 'missing template' error in presence of typos

2019-08-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added a project: clang.
ilya-biryukov added a parent revision: D65591: [AST] Add a flag indicating if 
any subexpression had errors.

Suppress those diagnostics if lhs of a member expression contains
errors. Typo correction produces dependent expressions even in
non-template code, that led to spurious diagnostics before.

Also includes a small refactoring to improve code calling
ParseUnqualifiedId and ParseOptionalCXXScopeSpecifier in the common case
when ObjectType is null. This allows to avoid boiler plate required to
propagate whether lhs of a member expression had any errors in the
common code path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65592

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/dependent-typos-recovery.cpp

Index: clang/test/SemaTemplate/dependent-typos-recovery.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/dependent-typos-recovery.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// There should be no extra errors about missing 'template' keywords.
+struct B {
+  template 
+  int f(){};
+} builder;// expected-note 2{{'builder' declared here}}
+
+auto a = bilder.f(); // expected-error{{undeclared identifier 'bilder'; did you mean}}
+auto b = (*(+0)).f(); // expected-error{{undeclared identifier 'bilder'; did you mean}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -143,14 +143,11 @@
   return false;
 }
 
-TemplateNameKind Sema::isTemplateName(Scope *S,
-  CXXScopeSpec ,
-  bool hasTemplateKeyword,
-  const UnqualifiedId ,
-  ParsedType ObjectTypePtr,
-  bool EnteringContext,
-  TemplateTy ,
-  bool ) {
+TemplateNameKind
+Sema::isTemplateName(Scope *S, CXXScopeSpec , bool hasTemplateKeyword,
+ const UnqualifiedId , ParsedType ObjectTypePtr,
+ bool EnteringContext, TemplateTy ,
+ bool ) {
   assert(getLangOpts().CPlusPlus && "No template names in C!");
 
   DeclarationName TName;
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1576,7 +1576,7 @@
 
   CXXScopeSpec SS;
   if (getLangOpts().CPlusPlus &&
-  ParseOptionalCXXScopeSpecifier(SS, nullptr, EnteringContext))
+  ParseOptionalCXXScopeSpecifier(SS, EnteringContext))
 return ANK_Error;
 
   if (Tok.isNot(tok::identifier) || SS.isInvalid()) {
@@ -1779,9 +1779,8 @@
 //simple-template-id
 SourceLocation TypenameLoc = ConsumeToken();
 CXXScopeSpec SS;
-if (ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/nullptr,
-   /*EnteringContext=*/false, nullptr,
-   /*IsTypename*/ true))
+if (ParseOptionalCXXScopeSpecifier(SS, /*EnteringContext=*/false, nullptr,
+   /*IsTypename=*/true))
   return true;
 if (!SS.isSet()) {
   if (Tok.is(tok::identifier) || Tok.is(tok::annot_template_id) ||
@@ -1852,7 +1851,7 @@
 
   CXXScopeSpec SS;
   if (getLangOpts().CPlusPlus)
-if (ParseOptionalCXXScopeSpecifier(SS, nullptr, /*EnteringContext*/false))
+if (ParseOptionalCXXScopeSpecifier(SS, /*EnteringContext*/ false))
   return true;
 
   return TryAnnotateTypeOrScopeTokenAfterScopeSpec(SS, !WasScopeAnnotation);
@@ -1984,7 +1983,7 @@
  "Cannot be a type or scope token!");
 
   CXXScopeSpec SS;
-  if (ParseOptionalCXXScopeSpecifier(SS, nullptr, EnteringContext))
+  if (ParseOptionalCXXScopeSpecifier(SS, EnteringContext))
 return true;
   if (SS.isEmpty())
 return false;
@@ -2093,8 +2092,7 @@
 
   // Parse nested-name-specifier.
   if (getLangOpts().CPlusPlus)
-ParseOptionalCXXScopeSpecifier(Result.SS, nullptr,
-   /*EnteringContext=*/false);
+ParseOptionalCXXScopeSpecifier(Result.SS, /*EnteringContext=*/false);
 
   // Check nested-name specifier.
   if (Result.SS.isInvalid()) {
@@ -2105,8 +2103,8 @@
   // Parse the unqualified-id.
   SourceLocation TemplateKWLoc; // FIXME: parsed, but unused.
   if 

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

2019-08-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added a subscriber: jfb.
Herald added a project: clang.
ilya-biryukov added a child revision: D65592: [Parser] Avoid spurious 'missing 
template' error in presence of typos.

The only subexpression that is considered an error now is TypoExpr, but
we plan to add expressions with errors to improve editor tooling on broken
code. We intend to use the same mechanism to guard against spurious
diagnostics on those as well.

See the follow-up revision for an actual usage of the flag.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65591

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprObjC.cpp

Index: clang/lib/AST/ExprObjC.cpp
===
--- clang/lib/AST/ExprObjC.cpp
+++ clang/lib/AST/ExprObjC.cpp
@@ -26,7 +26,7 @@
 ObjCArrayLiteral::ObjCArrayLiteral(ArrayRef Elements, QualType T,
ObjCMethodDecl *Method, SourceRange SR)
 : Expr(ObjCArrayLiteralClass, T, VK_RValue, OK_Ordinary, false, false,
-   false, false),
+   false, false, /*ContainsErros*/ false),
   NumElements(Elements.size()), Range(SR), ArrayWithObjectsMethod(Method) {
   Expr **SaveElements = getElements();
   for (unsigned I = 0, N = Elements.size(); I != N; ++I) {
@@ -36,6 +36,8 @@
   ExprBits.InstantiationDependent = true;
 if (Elements[I]->containsUnexpandedParameterPack())
   ExprBits.ContainsUnexpandedParameterPack = true;
+if (Elements[I]->containsErrors())
+  ExprBits.ContainsErrors = true;
 
 SaveElements[I] = Elements[I];
   }
@@ -60,7 +62,7 @@
  ObjCMethodDecl *method,
  SourceRange SR)
 : Expr(ObjCDictionaryLiteralClass, T, VK_RValue, OK_Ordinary, false, false,
-   false, false),
+   false, false, false),
   NumElements(VK.size()), HasPackExpansions(HasPackExpansions), Range(SR),
   DictWithObjectsMethod(method) {
   KeyValuePair *KeyValues = getTrailingObjects();
@@ -77,6 +79,8 @@
 (VK[I].Key->containsUnexpandedParameterPack() ||
  VK[I].Value->containsUnexpandedParameterPack()))
   ExprBits.ContainsUnexpandedParameterPack = true;
+if (VK[I].Key->containsErrors() || VK[I].Value->containsErrors())
+  ExprBits.ContainsErrors = true;
 
 KeyValues[I].Key = VK[I].Key;
 KeyValues[I].Value = VK[I].Value;
@@ -130,7 +134,7 @@
 : Expr(ObjCMessageExprClass, T, VK, OK_Ordinary,
/*TypeDependent=*/false, /*ValueDependent=*/false,
/*InstantiationDependent=*/false,
-   /*ContainsUnexpandedParameterPack=*/false),
+   /*ContainsUnexpandedParameterPack=*/false, /*ContainsErrors=*/false),
   SelectorOrMethod(
   reinterpret_cast(Method ? Method : Sel.getAsOpaquePtr())),
   Kind(IsInstanceSuper ? SuperInstance : SuperClass),
@@ -150,7 +154,7 @@
  SourceLocation RBracLoc, bool isImplicit)
 : Expr(ObjCMessageExprClass, T, VK, OK_Ordinary, T->isDependentType(),
T->isDependentType(), T->isInstantiationDependentType(),
-   T->containsUnexpandedParameterPack()),
+   T->containsUnexpandedParameterPack(), /*ContainsErrors*/ false),
   SelectorOrMethod(
   reinterpret_cast(Method ? Method : Sel.getAsOpaquePtr())),
   Kind(Class), HasMethod(Method != nullptr), IsDelegateInitCall(false),
@@ -168,7 +172,8 @@
 : Expr(ObjCMessageExprClass, T, VK, OK_Ordinary,
Receiver->isTypeDependent(), Receiver->isTypeDependent(),
Receiver->isInstantiationDependent(),
-   Receiver->containsUnexpandedParameterPack()),
+   Receiver->containsUnexpandedParameterPack(),
+   Receiver->containsErrors()),
   SelectorOrMethod(
   reinterpret_cast(Method ? Method : Sel.getAsOpaquePtr())),
   Kind(Instance), HasMethod(Method != nullptr), IsDelegateInitCall(false),
@@ -191,6 +196,8 @@
   ExprBits.InstantiationDependent = true;
 if (Args[I]->containsUnexpandedParameterPack())
   ExprBits.ContainsUnexpandedParameterPack = true;
+if (Args[I]->containsErrors())
+  ExprBits.ContainsErrors = true;
 
 MyArgs[I] = Args[I];
   }
Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -104,7 +104,7 @@
SourceRange DirectInitRange)
 : Expr(CXXNewExprClass, Ty, VK_RValue, OK_Ordinary, Ty->isDependentType(),
Ty->isDependentType(), Ty->isInstantiationDependentType(),
-   Ty->containsUnexpandedParameterPack()),
+   

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

2019-08-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

This really needs a test, likely an astdump-one will show it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65589



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


[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

2019-08-01 Thread Anton Bikineev via Phabricator via cfe-commits
AntonBikineev created this revision.
AntonBikineev added reviewers: rsmith, klimek.
AntonBikineev added a project: clang.

Interestingly enough, but this wasn't caught in tests. I wonder if it's 
possible to write a test case for it.


Repository:
  rC Clang

https://reviews.llvm.org/D65589

Files:
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D65582: IR: accept and print numbered %N names for function args

2019-08-01 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert resigned from this revision.
jdoerfert added a comment.

I like the idea but I am not the right person to review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65582



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


[PATCH] D65573: Add User docs for ASTImporter

2019-08-01 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added a comment.

Thanks, the extra documentation is highly appreciated!




Comment at: clang/docs/LibASTImporter.rst:19
+``ASTContext`` holds long-lived AST nodes (such as types and decls) that can 
be referred to throughout the semantic analysis of a file.
+There are cases when we would like to work with more than one ``ASTContext``.
+For example, we'd like to parse multiple different files inside the same Clang 
tool.

Stylistic note: usually LLVM documentation is written avoiding "we". So for 
example:

```
In some cases it is preferable to work with more than one ``ASTContext``, for 
example when parsing multiple files inside one Clang-based tool.
```



Comment at: clang/docs/LibASTImporter.rst:33
+By importing one AST node we copy that node into the destination 
``ASTContext``.
+Why do we have to copy the node?
+Isn't enough just to insert the pointer to that node into the destination 
context?

Importing ... copies that node ...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65573



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


[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked 2 inline comments as done.
Charusso added inline comments.



Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:486
   const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx,
- LocationContext const *Parent,
- const Stmt *S,

Deadlines, eh?



Comment at: clang/test/Analysis/loop-unrolling.cpp:371
   for (int i = 0; i < 9; i++) {
-#ifdef ANALYZER_CM_Z3
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+#ifdef DFS
+clang_analyzer_numTimesReached(); // expected-warning {{18}}

The `ninja check-clang-analyzer-z3` is not really working and we definitely 
have different results based on the traversal, so I just swapped that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65587



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


[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

2019-08-01 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, zzheng, szepet, baloghadamsoftware, xazax.hun.

With this addition we can distinguish between `StackSpaceRegions`,
as they hold `StackFrameContexts` part of their identity.

Thanks to Artem Dergachev for the great idea!


Repository:
  rC Clang

https://reviews.llvm.org/D65587

Files:
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/loop-block-counts.c
  clang/test/Analysis/loop-unrolling.cpp
  clang/test/Analysis/stack-frame-context-revision.cpp

Index: clang/test/Analysis/stack-frame-context-revision.cpp
===
--- /dev/null
+++ clang/test/Analysis/stack-frame-context-revision.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -verify %s
+
+// expected-no-diagnostics:
+// From now the profile of the 'StackFrameContext' also contains the
+// 'NodeBuilderContext::blockCount()'. With this addition we can distinguish
+// between the 'StackArgumentsSpaceRegion' of the 'P' arguments being different
+// on every iteration.
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+template 
+struct SmarterPointer {
+  PointerTy getFromVoidPointer(void *P) const {
+return static_cast(P);
+  }
+
+  PointerTy getPointer() const {
+return getFromVoidPointer(reinterpret_cast(Value));
+  }
+
+  intptr_t Value = 13;
+};
+
+struct Node {
+  SmarterPointer Pred;
+};
+
+void test(Node *N) {
+  while (N) {
+SmarterPointer Next = N->Pred;
+delete N;
+
+N = Next.getPointer();
+// no-warning: 'Use of memory after it is freed' was here as the same
+// 'StackArgumentsSpaceRegion' purged out twice as 'P'.
+  }
+}
Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -347,9 +347,9 @@
 int simple_unknown_bound_loop() {
   for (int i = 2; i < getNum(); i++) {
 #ifdef DFS
-clang_analyzer_numTimesReached(); // expected-warning {{10}}
+clang_analyzer_numTimesReached(); // expected-warning {{16}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+clang_analyzer_numTimesReached(); // expected-warning {{8}}
 #endif
   }
   return 0;
@@ -368,10 +368,10 @@
 int nested_inlined_no_unroll1() {
   int k;
   for (int i = 0; i < 9; i++) {
-#ifdef ANALYZER_CM_Z3
-clang_analyzer_numTimesReached(); // expected-warning {{13}}
+#ifdef DFS
+clang_analyzer_numTimesReached(); // expected-warning {{18}}
 #else
-clang_analyzer_numTimesReached(); // expected-warning {{15}}
+clang_analyzer_numTimesReached(); // expected-warning {{14}}
 #endif
 k = simple_unknown_bound_loop();  // reevaluation without inlining, splits the state as well
   }
Index: clang/test/Analysis/loop-block-counts.c
===
--- clang/test/Analysis/loop-block-counts.c
+++ clang/test/Analysis/loop-block-counts.c
@@ -12,7 +12,7 @@
   for (int i = 0; i < 2; ++i)
 callee([i]);
   // FIXME: Should be UNKNOWN.
-  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{FALSE}}
 }
 
 void loopWithCall() {
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -451,9 +451,8 @@
   // Construct a new stack frame for the callee.
   AnalysisDeclContext *CalleeADC = AMgr.getAnalysisDeclContext(D);
   const StackFrameContext *CalleeSFC =
-CalleeADC->getStackFrame(ParentOfCallee, CallE,
- currBldrCtx->getBlock(),
- currStmtIdx);
+  CalleeADC->getStackFrame(ParentOfCallee, CallE, currBldrCtx->getBlock(),
+   currBldrCtx->blockCount(), currStmtIdx);
 
   CallEnter Loc(CallE, CalleeSFC, CurLC);
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -323,7 +323,8 @@
   CallEventManager  = getStateManager().getCallEventManager();
   SVal V = UnknownVal();
   auto getArgLoc = [&](CallEventRef<> Caller) -> Optional {
-const LocationContext *FutureSFC = Caller->getCalleeStackFrame();
+

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

2019-08-01 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3859
   ToTypeSourceInfo, D->getStorageClass(),
   /*DefaultArg*/ nullptr))
 return ToParm;

This should be `DefaultArg` now?



Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:135
+
+struct DefParmContext {
+  static const int I;

martong wrote:
> Perhaps we could write `Default` instead of `Def`.
+1 to this and the name suggestion below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65577



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


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

2019-08-01 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld removed a reviewer: Hahnfeld.
Hahnfeld added inline comments.
This revision is now accepted and ready to land.



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

gtbercea wrote:
> ABataev wrote:
> > Why?
> If the pointee has been mapped as TO/FROM/etc already no need to map it 
> TO/FROM/etc again.
I'm not sure if there's a test for this. I can't verify right now, so I trust 
you that it's covered.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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


[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

2019-08-01 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:84
+DependencyScanningFilesystemSharedCache() {
+  NumShards = 8;
+  CacheShards = llvm::make_unique(NumShards);

arphaman wrote:
> aganea wrote:
> > This line needs a comment. Is this value based on empirical results across 
> > different hardwares? (I can't recall your answer at the LLVM conf) I am 
> > wondering what would be the good strategy here? The more cores/HT in your 
> > PC, the more chances that you'll hit the same shard, thus locking. But the 
> > bigger we make this value, the more `StringMaps` we will have, and more 
> > cache misses possibly.
> > Should it be something like `llvm::hardware_concurrency() / some_fudge`? 
> > It'd be interesting to subsequently profile on high core count machines (or 
> > maybe you have done that).
> I rewrote it to use a heuristic we settled on after doing empirical testing 
> on an 18 core / 36 thread machine ( max(2, thread_count / 4) ), and added a 
> comment. This was the number `9` (36 / 4) I mentioned at the talk, so we only 
> got to it because of a heuristic. I think now after some SDK/OS updates the 
> effect from adding more shards is less pronounced, so it mostly flatlines 
> past some number between 5-10. A better heuristic would probably be OS 
> specific to take the cost of lock contention into account.
> 
> Note that the increased number of shards does not increase the number of 
> cache misses, because the shard # is determined by the filename (we don't 
> actually have global cache misses, as the cache is designed to have only one 
> miss per file when it's first accessed)! It's just that an increased number 
> of shards doesn't improve performance after hitting some specific limit, so 
> we want to find a point where we can taper it off.
> 
> It would still be definitely interesting to profile it on other high core 
> machines with different OSes to see if it's a reasonable heuristic for those 
> scenarios too.
I'll give it a try on Windows 10, one project here has a large codebase and 
some wild machines to test on.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:156
+/// this subclass.
+class MinimizedVFSFile final : public llvm::vfs::File {
+public:

This makes only a short-lived objects, is that right? Just during the call to 
`CachedFileSystemEntry::createFileEntry`?



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:94
+  // Add any filenames that were explicity passed in the build settings and
+  // that might be might be opened, as we want to ensure we don't run 
source
+  // minimization on them.

"might be" twice.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:148
+  RealFS = new ProxyFileSystemWithoutChdir(llvm::vfs::getRealFileSystem());
+  if (Service.getMode() == ScanningMode::MinimizedSourcePreprocessing)
+DepFS = new DependencyScanningFilesystem(Service.getSharedCache(), RealFS);

arphaman wrote:
> aganea wrote:
> > Can we not use caching all the time?
> We want to have a mode where it's as close to the regular `clang -E` 
> invocation as possible for correctness CI and testing. We also haven't 
> evaluated if the cost of keeping non-minimized sources in memory, so it might 
> be too expensive for practical use? I can add a third option that caches but 
> doesn't minimize though as a follow-up patch though 
> 
Yes that would be nice. As for the size taken in RAM, it would be only .H 
files, right? For Clang+LLVM+LLD I'm counting about 40 MB. But indeed with a 
large project, that would be about 1.5 GB of .H. Although I doubt all these 
files will be loaded at once in memory (I'll check)

As for the usage: Fastbuild works like distcc (plain mode, not pump) so we were 
also planning on extracting the fully preprocessed output, not only the 
dependencies. There's one use-case where we need to preprocess locally, then 
send the preprocessed output remotely for compilation. And another use-case 
where we only want to extract the dependency list, compute a digest, to 
retrieve the OBJ from a network cache.


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

https://reviews.llvm.org/D63907



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


[PATCH] D65564: Improve raw_ostream so that you can "write" colors using operator<

2019-08-01 Thread Andrea Di Biagio via Phabricator via cfe-commits
andreadb accepted this revision.
andreadb added a comment.
This revision is now accepted and ready to land.

LGTM too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65564



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


[PATCH] D65582: IR: accept and print numbered %N names for function args

2019-08-01 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.
Herald added a subscriber: wuzish.

+1 for doing this. I started looking at fixing this when I modified the printer 
to print proper labels for numbered basic-blocks (instead of comments), but I 
didn't do so because of the amount of test churn was off-putting.

I think that after this change, there's only one local entity left which uses a 
local-value-number but doesn't print it: the entry block. That also would cause 
a quite-large amount of test-churn to add.




Comment at: llvm/lib/IR/AsmWriter.cpp:3561
+else
+  Out << "";
   }

I think you need a space before this string? Although, then shouldn't 
llvm/unittests/IR/AsmWriterTest.cpp be failing? (it has a space there...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65582



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


[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

2019-08-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5654-5656
 def warn_addition_in_bitshift : Warning<
   "operator '%0' has lower precedence than '%1'; "
+  "'%1' will be evaluated first">, InGroup, DefaultIgnore;

MaskRay wrote:
> aaron.ballman wrote:
> > MaskRay wrote:
> > > rsmith wrote:
> > > > Do you have evidence that this warning has a significant false-positive 
> > > > rate? In my experience it's very common for people to think of `<<` as 
> > > > being a multiplication-like operator and be surprised when it turns out 
> > > > to have lower precedence than addition.
> > > warn_addition_in_bitshift has many false positives. Some results when 
> > > searching for `[-Wshift-op-parentheses]` and the most common diagnostic 
> > > `operator '<<' has lower precedence than '+'; '+' will be evaluated 
> > > first`: 
> > > 
> > > https://gitship.com/srutscher/pdp-6-emulator/blob/f41b119eee2f409ea519b15f3a76cfecb70c03d8/emu/Makefile
> > > https://www.openwall.com/lists/musl/2014/04/04/12
> > > https://salmonella-freebsd-x86-64.call-cc.org/chicken-4/clang/freebsd/x86-64/2018/06/21/salmonella-report/install/bvsp-spline.html
> > > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> > > https://github.com/rdkit/rdkit/issues/145
> > > ffmpeg 
> > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?format=multiple=191743
> > > https://clang.debian.net/logs/2015-03-25/fairymax_4.8v-1_unstable_clang.log
> > > 
> > Some of those look like true positives. For instance, the fix to 
> > https://github.com/rdkit/rdkit/issues/145 was 
> > https://github.com/rdkit/rdkit/commit/38ca41c8abc3f4429ae8df2ad79d3ff8b3fea0b6
> >  which looks like the warning behaved as intended. 
> > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> >  doesn't really have anything to do with the diagnostic.
> > 
> > FWIW, in most of the cases, I feel like parens would clarify the code 
> > (heck, they're already using parens in many of these cases). e.g.,
> > ```
> > fairymax.c:776:46: warning: operator '>>' has lower precedence than '+'; 
> > '+' will be evaluated first [-Wshift-op-parentheses]
> > MovesLeft = -(GamePtr+(Side==WHITE)>>1);
> >   ~~~^~~~
> > 
> > dbvspis.c:606:20: warning: operator '<<' has lower precedence than '+'; '+' 
> > will be evaluated first [-Wshift-op-parentheses]
> > i3 = i2 + (*np + 1 << 1);
> >^~~ ~~
> > ```
> > FWIW, I'm fine leaving this default on.
> > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422
> 
> Note  -Wno-shift-op-parentheses
> 
> > https://github.com/rdkit/rdkit/issues/145 which looks like the warning 
> > behaved as intended. 
> 
> Yes, this is a true positive. I just randomly searched for some cases, didn't 
> carefully verify them, sorry.
> 
> > FWIW, in most of the cases, I feel like parens would clarify the code 
> > (heck, they're already using parens in many of these cases). e.g.,
> 
> -Wparentheses certainly has its position and it does catch real errors. I 
> also agree that many people are in favor of it. However, there are as many 
> people who dislike it and add -Wno-parentheses to their build systems. Many 
> people complain that extraneous parentheses clutter the code, especially 
> multiple levels of parentheses.
> 
> The discrepancy between clang default and gcc default here is a bit 
> unfortunate. People porting software to clang make a lot of style changes. 
> They can mess up git blame, make bugfix backporting difficult, and so on.
> 
> The 3 subgroups of -Wparentheses have the most false positives. This is an 
> indicator that they should not belong to the set of default-on warnings. 
> People who use -Wall (a lot) will not notice the difference. I'm thinking if 
> -Wall didn't include these controversial warnings people would be happier 
> (clang -Wmost doesn't include -Wparentheses but unfortunately gcc doesn't 
> have -Wmost). It is also unfortunate -Wparentheses is all-or-nothing...
> -Wparentheses certainly has its position and it does catch real errors. I 
> also agree that many people are in favor of it. However, there are as many 
> people who dislike it and add -Wno-parentheses to their build systems. Many 
> people complain that extraneous parentheses clutter the code, especially 
> multiple levels of parentheses.

They don't seem to be complaining to our bug tracker -- I did not see any 
complaints about the behavior of `warn_addition_in_bitshift` (there were a few 
complaints about some of the other issues you're addressing in this patch 
though).

My concern is that we have plenty of evidence to demonstrate that users do not 
enable warnings that are off by default, and this warning does catch true 
positive issues that can be subtle and hard to track down without the warning. 
I'm not yet 

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

2019-08-01 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
ostannard updated this revision to Diff 212833.
ostannard added a comment.

- Add LTOPostLink metadata, instead of internalising vcall visibility at LTO 
time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  clang/test/CodeGenCXX/virtual-function-elimination.cpp
  clang/test/Driver/virtual-function-elimination.cpp
  llvm/docs/LangRef.rst
  llvm/docs/TypeMetadata.rst
  llvm/include/llvm/Analysis/TypeMetadataUtils.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/GlobalObject.h
  llvm/include/llvm/Transforms/IPO/GlobalDCE.h
  llvm/lib/Analysis/TypeMetadataUtils.cpp
  llvm/lib/IR/Metadata.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/LTO/ARM/lto-linking-metadata.ll
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-pre-lto.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/Internalize/vcall-visibility.ll

Index: llvm/test/Transforms/Internalize/vcall-visibility.ll
===
--- /dev/null
+++ llvm/test/Transforms/Internalize/vcall-visibility.ll
@@ -0,0 +1,64 @@
+; RUN: opt < %s -internalize -S | FileCheck %s
+
+%struct.A = type { i32 (...)** }
+%struct.B = type { i32 (...)** }
+%struct.C = type { i32 (...)** }
+
+; Class A has default visibility, so has no !vcall_visibility metadata before
+; or after LTO.
+; CHECK-NOT: @_ZTV1A = {{.*}}!vcall_visibility
+@_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0, !type !1
+
+; Class B has hidden visibility but public LTO visibility, so has no
+; !vcall_visibility metadata before or after LTO.
+; CHECK-NOT: @_ZTV1B = {{.*}}!vcall_visibility
+@_ZTV1B = hidden unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.B*)* @_ZN1B3fooEv to i8*)] }, align 8, !type !2, !type !3
+
+; Class C has hidden visibility, so the !vcall_visibility metadata is set to 1
+; (linkage unit) before LTO, and 2 (translation unit) after LTO.
+; CHECK: @_ZTV1C ={{.*}}!vcall_visibility [[MD_TU_VIS:![0-9]+]]
+@_ZTV1C = hidden unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.C*)* @_ZN1C3fooEv to i8*)] }, align 8, !type !4, !type !5, !vcall_visibility !6
+
+; Class D has translation unit visibility before LTO, and this is not changed
+; by LTO.
+; CHECK: @_ZTVN12_GLOBAL__N_11DE = {{.*}}!vcall_visibility [[MD_TU_VIS:![0-9]+]]
+@_ZTVN12_GLOBAL__N_11DE = internal unnamed_addr constant { [3 x i8*] } zeroinitializer, align 8, !type !7, !type !9, !vcall_visibility !11
+
+define dso_local void @_ZN1A3fooEv(%struct.A* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden void @_ZN1B3fooEv(%struct.B* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden void @_ZN1C3fooEv(%struct.C* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden noalias nonnull i8* @_Z6make_dv() {
+entry:
+  %call = tail call i8* @_Znwm(i64 8) #3
+  %0 = bitcast i8* %call to i32 (...)***
+  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [3 x i8*] }, { [3 x i8*] }* @_ZTVN12_GLOBAL__N_11DE, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 8
+  ret i8* %call
+}
+
+declare dso_local noalias nonnull i8* @_Znwm(i64)
+
+; CHECK: [[MD_TU_VIS]] = !{i64 2}
+!0 = !{i64 16, !"_ZTS1A"}
+!1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
+!2 = !{i64 16, !"_ZTS1B"}
+!3 = !{i64 16, !"_ZTSM1BFvvE.virtual"}
+!4 = !{i64 16, !"_ZTS1C"}
+!5 = !{i64 16, !"_ZTSM1CFvvE.virtual"}
+!6 = !{i64 1}
+!7 = !{i64 16, !8}
+!8 = distinct !{}
+!9 = !{i64 16, !10}
+!10 = distinct !{}
+!11 = !{i64 2}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- /dev/null
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -0,0 +1,55 @@
+; RUN: opt < %s -globaldce -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+declare dso_local noalias nonnull i8* 

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

2019-08-01 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 added a comment.

Looks fine to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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


[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

2019-08-01 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM, though I have some comments.




Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:135
+
+struct DefParmContext {
+  static const int I;

Perhaps we could write `Default` instead of `Def`.



Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:144
+
+int testDefParmIncompleteImport(int I) {
+  return fDefParm(I);

`testImportOfIncompleteDefaultParm` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65577



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


[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Nice! This makes https://bugs.llvm.org/show_bug.cgi?id=42524#c3 easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


[PATCH] D65405: [Parser] Use special definition for pragma annotations

2019-08-01 Thread Serge Pavlov via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367575: [Parser] Use special definition for pragma 
annotations (authored by sepavloff, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65405?vs=212186=212822#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65405

Files:
  cfe/trunk/include/clang/Basic/TokenKinds.def
  cfe/trunk/include/clang/Basic/TokenKinds.h
  cfe/trunk/lib/Basic/TokenKinds.cpp

Index: cfe/trunk/lib/Basic/TokenKinds.cpp
===
--- cfe/trunk/lib/Basic/TokenKinds.cpp
+++ cfe/trunk/lib/Basic/TokenKinds.cpp
@@ -45,3 +45,13 @@
   }
   return nullptr;
 }
+
+bool tok::isPragmaAnnotation(TokenKind Kind) {
+  switch (Kind) {
+#define PRAGMA_ANNOTATION(X) case annot_ ## X: return true;
+#include "clang/Basic/TokenKinds.def"
+  default:
+break;
+  }
+  return false;
+}
Index: cfe/trunk/include/clang/Basic/TokenKinds.h
===
--- cfe/trunk/include/clang/Basic/TokenKinds.h
+++ cfe/trunk/include/clang/Basic/TokenKinds.h
@@ -98,6 +98,9 @@
   return false;
 }
 
+/// Return true if this is an annotation token representing a pragma.
+bool isPragmaAnnotation(TokenKind K);
+
 }  // end namespace tok
 }  // end namespace clang
 
Index: cfe/trunk/include/clang/Basic/TokenKinds.def
===
--- cfe/trunk/include/clang/Basic/TokenKinds.def
+++ cfe/trunk/include/clang/Basic/TokenKinds.def
@@ -68,6 +68,9 @@
 #ifndef ANNOTATION
 #define ANNOTATION(X) TOK(annot_ ## X)
 #endif
+#ifndef PRAGMA_ANNOTATION
+#define PRAGMA_ANNOTATION(X) ANNOTATION(X)
+#endif
 
 //===--===//
 // Preprocessor keywords.
@@ -729,103 +732,103 @@
 // Annotation for #pragma unused(...)
 // For each argument inside the parentheses the pragma handler will produce
 // one 'pragma_unused' annotation token followed by the argument token.
-ANNOTATION(pragma_unused)
+PRAGMA_ANNOTATION(pragma_unused)
 
 // Annotation for #pragma GCC visibility...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_vis)
+PRAGMA_ANNOTATION(pragma_vis)
 
 // Annotation for #pragma pack...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_pack)
+PRAGMA_ANNOTATION(pragma_pack)
 
 // Annotation for #pragma clang __debug parser_crash...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_parser_crash)
+PRAGMA_ANNOTATION(pragma_parser_crash)
 
 // Annotation for #pragma clang __debug captured...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_captured)
+PRAGMA_ANNOTATION(pragma_captured)
 
 // Annotation for #pragma clang __debug dump...
 // The lexer produces these so that the parser and semantic analysis can
 // look up and dump the operand.
-ANNOTATION(pragma_dump)
+PRAGMA_ANNOTATION(pragma_dump)
 
 // Annotation for #pragma ms_struct...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_msstruct)
+PRAGMA_ANNOTATION(pragma_msstruct)
 
 // Annotation for #pragma align...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_align)
+PRAGMA_ANNOTATION(pragma_align)
 
 // Annotation for #pragma weak id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weak)
+PRAGMA_ANNOTATION(pragma_weak)
 
 // Annotation for #pragma weak id = id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weakalias)
+PRAGMA_ANNOTATION(pragma_weakalias)
 
 // Annotation for #pragma redefine_extname...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_redefine_extname)
+PRAGMA_ANNOTATION(pragma_redefine_extname)
 
 // Annotation for #pragma STDC FP_CONTRACT...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fp_contract)
+PRAGMA_ANNOTATION(pragma_fp_contract)
 
 // Annotation for #pragma STDC FENV_ACCESS
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fenv_access)
+PRAGMA_ANNOTATION(pragma_fenv_access)
 
 // Annotation for #pragma pointers_to_members...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_ms_pointers_to_members)

r367575 - [Parser] Use special definition for pragma annotations

2019-08-01 Thread Serge Pavlov via cfe-commits
Author: sepavloff
Date: Thu Aug  1 08:15:10 2019
New Revision: 367575

URL: http://llvm.org/viewvc/llvm-project?rev=367575=rev
Log:
[Parser] Use special definition for pragma annotations

Previously pragma annotation tokens were described as any other
annotations in TokenKinds.def. This change introduces special macro
PRAGMA_ANNOTATION for the pragma descriptions. It allows implementing
checks that deal with pragma annotations only.

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

Modified:
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Basic/TokenKinds.h
cfe/trunk/lib/Basic/TokenKinds.cpp

Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=367575=367574=367575=diff
==
--- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
+++ cfe/trunk/include/clang/Basic/TokenKinds.def Thu Aug  1 08:15:10 2019
@@ -68,6 +68,9 @@
 #ifndef ANNOTATION
 #define ANNOTATION(X) TOK(annot_ ## X)
 #endif
+#ifndef PRAGMA_ANNOTATION
+#define PRAGMA_ANNOTATION(X) ANNOTATION(X)
+#endif
 
 
//===--===//
 // Preprocessor keywords.
@@ -729,103 +732,103 @@ ANNOTATION(decltype) // annotation f
 // Annotation for #pragma unused(...)
 // For each argument inside the parentheses the pragma handler will produce
 // one 'pragma_unused' annotation token followed by the argument token.
-ANNOTATION(pragma_unused)
+PRAGMA_ANNOTATION(pragma_unused)
 
 // Annotation for #pragma GCC visibility...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_vis)
+PRAGMA_ANNOTATION(pragma_vis)
 
 // Annotation for #pragma pack...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_pack)
+PRAGMA_ANNOTATION(pragma_pack)
 
 // Annotation for #pragma clang __debug parser_crash...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_parser_crash)
+PRAGMA_ANNOTATION(pragma_parser_crash)
 
 // Annotation for #pragma clang __debug captured...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_captured)
+PRAGMA_ANNOTATION(pragma_captured)
 
 // Annotation for #pragma clang __debug dump...
 // The lexer produces these so that the parser and semantic analysis can
 // look up and dump the operand.
-ANNOTATION(pragma_dump)
+PRAGMA_ANNOTATION(pragma_dump)
 
 // Annotation for #pragma ms_struct...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_msstruct)
+PRAGMA_ANNOTATION(pragma_msstruct)
 
 // Annotation for #pragma align...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_align)
+PRAGMA_ANNOTATION(pragma_align)
 
 // Annotation for #pragma weak id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weak)
+PRAGMA_ANNOTATION(pragma_weak)
 
 // Annotation for #pragma weak id = id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weakalias)
+PRAGMA_ANNOTATION(pragma_weakalias)
 
 // Annotation for #pragma redefine_extname...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_redefine_extname)
+PRAGMA_ANNOTATION(pragma_redefine_extname)
 
 // Annotation for #pragma STDC FP_CONTRACT...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fp_contract)
+PRAGMA_ANNOTATION(pragma_fp_contract)
 
 // Annotation for #pragma STDC FENV_ACCESS
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fenv_access)
+PRAGMA_ANNOTATION(pragma_fenv_access)
 
 // Annotation for #pragma pointers_to_members...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_ms_pointers_to_members)
+PRAGMA_ANNOTATION(pragma_ms_pointers_to_members)
 
 // Annotation for #pragma vtordisp...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_ms_vtordisp)
+PRAGMA_ANNOTATION(pragma_ms_vtordisp)
 
 // Annotation for all microsoft #pragmas...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_ms_pragma)
+PRAGMA_ANNOTATION(pragma_ms_pragma)
 
 // Annotation for #pragma OPENCL EXTENSION...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_opencl_extension)
+PRAGMA_ANNOTATION(pragma_opencl_extension)
 
 // Annotations for OpenMP pragma directives 

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

2019-08-01 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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


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

2019-08-01 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

@Hahnfeld I have merged the tests from the previous patch as per Alexey's 
suggestion - with minor changes to make them pass. Let me know if this now 
addresses your previous comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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


  1   2   >