date:20190327

[PATCH] D59857: [analyzer] PR37501: Disable the assertion for reverse-engineering logical op short circuits.

2019-03-27 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

There is no connection with my reverse-engineering reverted patch: 
https://reviews.llvm.org/D57410?id=184992 ? It evaluates the left and the right 
side and may it could help.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59857



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

2019-03-27 Thread Don Hinton via Phabricator via cfe-commits

hintonda updated this revision to Diff 192416.
hintonda added a comment.

- Added dyn_cast and dyn_cast_or_null, and provided fixit's for all of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,102 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y, Z *LongVarName) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast_or_null(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (y && isa(y))
+
+  if (dyn_cast_or_null(LongVarName->bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast_or_null<> not used.*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (LongVarName->bar() && isa(LongVarName->bar()))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast_or_null(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (y && isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast_or_null(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (y && isa(y))
+
+  do {
+break;
+  } while (dyn_cast_or_null(LongVarName->bar()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast_or_null<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (LongVarName->bar() && isa(LongVarName->bar()))
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-27 Thread Anton Afanasyev via Phabricator via cfe-commits

anton-afanasyev marked 3 inline comments as done.
anton-afanasyev added inline comments.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

takuto.ikuta wrote:
> Remove StringRef?
I use `StringRef("")` to explicitly cast to one of overloaded 
`TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, 
fuction_ref(std::string()))`.



Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+*OS << "{ \"traceEvents\": [\n";
+

takuto.ikuta wrote:
> Don't we want to use json utility for this?
> https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
> 
This implementation looks compact and fast. I've read proposal for this library 
https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4 
, it's recent, so I'm not sure it's stable and speed optimized. Do you actually 
can advise it? I can do it in the next refactor commit as well.



Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+  if (isPrint(C)) {
+OS += C;

takuto.ikuta wrote:
> include StringExtras.h for this?
Should one do it? It's already implicitly included.


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

https://reviews.llvm.org/D58675



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

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

2019-03-27 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a reviewer: aaron.ballman.
lebedev.ri added a comment.

Please always upload all patches with full context (`-U9`)
I'm not sure why we want this? What is wrong with simply applying clang-tidy 
twice?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59859



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

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

2019-03-27 Thread Csaba Dabis via Phabricator via cfe-commits

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

I like the idea to put connecting stuff together in the same file to create 
more understandable code. LGTM.




Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.h:8
+//
+//===--===//
+//

Outdated header-blurb.



Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.h:81
+
+void dumpTaint(ProgramStateRef State);
+

We left the `ProgramState::dump()' context so what about just `dump()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59861



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-03-27 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

It is best to error-out early.
Could you please try this instead:




Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:39
 }
 
 } // namespace

```
AST_MATCHER_P(CXXNewExpr, hasInitializationStyle,
  CXXNewExpr::InitializationStyle, IS) {
  return Node.getInitializationStyle() == IS;
};
```



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:94
  equalsBoundNode(PointerType),
  CanCallCtor)
   .bind(NewExpression)),

```
CanCallCtor, anyOf(unless(IgnoreListInit), 
unless(hasInitializationStyle(CXXNewExpr::ListInit
```



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:104
   callee(cxxMethodDecl(hasName("reset"))),
   hasArgument(0, cxxNewExpr(CanCallCtor).bind(NewExpression)),
   unless(isInTemplateInstantiation()))

```
hasArgument(0, cxxNewExpr(CanCallCtor, anyOf(unless(IgnoreListInit), 
unless(hasInitializationStyle(CXXNewExpr::ListInit.bind(NewExpression)),
```


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

https://reviews.llvm.org/D55044



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas created this revision.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.

The bugprone-too-small-loop-variable check often catches loop variables which 
can represent "big enough" values, so we don't actually need to worry about 
that this variable will overflow in a loop when the code iterates through a 
container. For example a 32 bit signed integer type's maximum value is 2 147 
483 647 and a container's size won't reach this maximum value in most of the 
cases.
So the idea of this option to allow the user to specify an upper limit (using 
magnitude bit of the integer type) to filter out those catches which are not 
interesting for the user, so he/she can focus on the more risky integer 
incompatibilities.
Next to the option I replaced the term "positive bits" to "magnitude bits" 
which seems a better naming both in the code and in the name of the new option.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59870

Files:
  clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp

Index: test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-too-small-loop-variable.MagnitudeBitsUpperLimit, \
+// RUN:   value: 31}]}" \
+// RUN:   -- --target=x86_64-linux
+
+unsigned long size() { return 294967296l; }
+
+void voidFilteredOutForLoop1() {
+  for (long i = 0; i < size(); ++i) {
+// no warning
+  }
+}
+
+void voidFilteredOutForLoop2() {
+  for (unsigned i = 0; i < size(); ++i) {
+// no warning
+  }
+}
+
+void voidCaughtForLoop1() {
+  for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidCaughtForLoop2() {
+  for (short i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
+  }
+}
Index: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
===
--- docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
+++ docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
@@ -27,3 +27,20 @@
 
 This algorithm works for small amount of objects, but will lead to freeze for a
 a larger user input.
+
+.. option:: MagnitudeBitsUpperLimit
+
+  Upper limit for the magnitue bits of the loop variable. If it's set the check
+  filters out those catches in which the loop variable's type has more magnitude
+  bit as the specified upper limit.
+  For example, if the user sets this attribute to 31, then a 32-bit ``unsigend int``
+  is ignored by the check, however a 32-bit ``int`` is not (A 32-bit ``signed int``
+  has 31 magnitude bits).
+
+.. code-block:: c++
+
+  int main() {
+long size = 294967296l;
+for (unsigned i = 0; i < size; ++i) {} // no warning
+for (int i = 0; i < size; ++i) {} // warning
+  }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -119,6 +119,10 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :bugprone-too-small-loop-variable
+  ` now supports
+  `MagnitudeBitsUpperLimit` option.
+
 - The :doc:`google-runtime-int `
   check has been disabled in Objective-C++.
 
Index: clang-tidy/bugprone/TooSmallLoopVariableCheck.h
===
--- clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ clang-tidy/bugprone/TooSmallLoopVariableCheck.h
@@ -29,10 +29,14 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-too-small-loop-variable.html
 class TooSmallLoopVariableCheck : public ClangTidyCheck {
 public:
-  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const unsigned MagnitudeBitsUpperLimit;
 };
 
 } // namespace bugprone
Index: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
=

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas added a comment.

I run the check with and without the option on llvm source code. Without the 
option I found 370 catches, while setting MagnitudeBitsUpperLimit to `30` I 
found only two catches:

  
/home/zolnai/libreoffice/clang/llvm-project/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:390:32:
 warning: loop variable has narrower type 'uint16_t' (aka 'unsigned short') 
than iteration's upper bound 'uint32_t' (aka 'unsigned int') 
[bugprone-too-small-loop-variable]
for (uint16_t StreamIdx = 0; StreamIdx < StreamCount; ++StreamIdx) {
 ^
  
  
/home/zolnai/libreoffice/clang/llvm-project/llvm/tools/llvm-pdbutil/StreamUtil.cpp:98:32:
 warning: loop variable has narrower type 'uint16_t' (aka 'unsigned short') 
than iteration's upper bound 'uint32_t' (aka 'unsigned int') 
[bugprone-too-small-loop-variable]
for (uint16_t StreamIdx = 0; StreamIdx < StreamCount; ++StreamIdx) {
 ^


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59870



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

[PATCH] D59857: [analyzer] PR37501: Disable the assertion for reverse-engineering logical op short circuits.

2019-03-27 Thread Alexander Kornienko via Phabricator via cfe-commits

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

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D59857



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas added a comment.

I found a project where the bugprone-too-small-loop-variable was used:
https://github.com/openMSX/openMSX/commit/55006063daffa90941d4c3f9b0034e494d9cf45a

As the commit message says for 32-bit integers the overflow never happens in 
practice.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59870



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

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

2019-03-27 Thread Borsik Gábor via Phabricator via cfe-commits

boga95 added a comment.

Thanks, it will make my changes more cleaner.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59861



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

r357061 - Revert the r348352 "[clang] - Simplify tools::SplitDebugName."

2019-03-27 Thread George Rimar via cfe-commits

Author: grimar
Date: Wed Mar 27 04:00:03 2019
New Revision: 357061

URL: http://llvm.org/viewvc/llvm-project?rev=357061&view=rev
Log:
Revert the r348352 "[clang] - Simplify tools::SplitDebugName."

This partially reverts the r348352 (https://reviews.llvm.org/D55006)
because of https://bugs.llvm.org/show_bug.cgi?id=41161.

I did not revert the test case file because it passes fine now.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=357061&r1=357060&r2=357061&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Mar 27 04:00:03 2019
@@ -4083,7 +4083,7 @@ void Clang::ConstructJob(Compilation &C,
   const char *SplitDWARFOut;
   if (SplitDWARF) {
 CmdArgs.push_back("-split-dwarf-file");
-SplitDWARFOut = SplitDebugName(Args, Output);
+SplitDWARFOut = SplitDebugName(Args, Input, Output);
 CmdArgs.push_back(SplitDWARFOut);
   }
 
@@ -6137,7 +6137,7 @@ void ClangAs::ConstructJob(Compilation &
   if ((getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split) &&
   (T.isOSLinux() || T.isOSFuchsia())) {
 CmdArgs.push_back("-split-dwarf-file");
-CmdArgs.push_back(SplitDebugName(Args, Output));
+CmdArgs.push_back(SplitDebugName(Args, Input, Output));
   }
 
   assert(Input.isFilename() && "Invalid input.");

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=357061&r1=357060&r2=357061&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Wed Mar 27 04:00:03 2019
@@ -816,18 +816,26 @@ bool tools::areOptimizationsEnabled(cons
   return false;
 }
 
-const char *tools::SplitDebugName(const ArgList &Args,
+const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
   const InputInfo &Output) {
-  SmallString<128> F(Output.isFilename()
- ? Output.getFilename()
- : llvm::sys::path::stem(Output.getBaseInput()));
-
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
 if (StringRef(A->getValue()) == "single")
-  return Args.MakeArgString(F);
+  return Args.MakeArgString(Output.getFilename());
 
-  llvm::sys::path::replace_extension(F, "dwo");
-  return Args.MakeArgString(F);
+  Arg *FinalOutput = Args.getLastArg(options::OPT_o);
+  if (FinalOutput && Args.hasArg(options::OPT_c)) {
+SmallString<128> T(FinalOutput->getValue());
+llvm::sys::path::replace_extension(T, "dwo");
+return Args.MakeArgString(T);
+  } else {
+// Use the compilation dir.
+SmallString<128> T(
+Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
+SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
+llvm::sys::path::replace_extension(F, "dwo");
+T += F;
+return Args.MakeArgString(F);
+  }
 }
 
 void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.h?rev=357061&r1=357060&r2=357061&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h Wed Mar 27 04:00:03 2019
@@ -62,7 +62,7 @@ void AddHIPLinkerScript(const ToolChain
 const Tool &T);
 
 const char *SplitDebugName(const llvm::opt::ArgList &Args,
-   const InputInfo &Output);
+   const InputInfo &Input, const InputInfo &Output);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
 const JobAction &JA, const llvm::opt::ArgList &Args,

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=357061&r1=357060&r2=357061&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Wed Mar 27 04:00:03 2019
@@ -826,7 +826,7 @@ void tools::gnutools::Assembler::Constru
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
   getToolChain().getTriple().isOSLinux())
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(Args, Outpu

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits

danzimm created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In https://bugs.llvm.org/show_bug.cgi?id=41206 we observe bad codegen when 
embedding a non-trivial C struct within a C struct. This is due to the fact 
that name mangling for non-trivial structs marks the two structs as identical. 
This diff contains a potential fix for this issue. I've included a test that 
exercises this for two scenarios. Please comment if there're any other 
scenarios I should consider.

Test Plan: I've included a unit test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59873

Files:
  clang/lib/CodeGen/CGNonTrivialStruct.cpp


Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -139,8 +139,8 @@
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  
|
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 


Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -139,8 +139,8 @@
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  |
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59874: [PR41247] Fixed parsing of private keyword in C++

2019-03-27 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia created this revision.
Anastasia added reviewers: rjmccall, jsji.
Herald added a subscriber: ebevhan.

In C++ `private` shouldn't be parsed as a valid address space keyword.

This only fixes C++ part, in OpenCL we should still prevent ICE on the reported 
code example, but I will fix it separately.

@jsji  thanks for filing the bug!


https://reviews.llvm.org/D59874

Files:
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseTentative.cpp
  test/SemaOpenCLCXX/private-access-specifier.cpp


Index: test/SemaOpenCLCXX/private-access-specifier.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/private-access-specifier.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -pedantic -verify -fsyntax-only
+
+struct B {
+virtual ~B() // expected-error{{expected ';' at end of declaration 
list}}
+private:
+void foo();
+};
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1410,8 +1410,13 @@
 // cv-qualifier
   case tok::kw_const:
   case tok::kw_volatile:
+return TPResult::True;
+
 // OpenCL address space qualifiers
   case tok::kw_private:
+if (!getLangOpts().OpenCL)
+  return TPResult::False;
+LLVM_FALLTHROUGH;
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4791,7 +4791,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4800,9 +4799,11 @@
   case tok::kw___read_only:
   case tok::kw___read_write:
   case tok::kw___write_only:
-
 return true;
 
+  case tok::kw_private:
+return getLangOpts().OpenCL;
+
   // C11 _Atomic
   case tok::kw__Atomic:
 return true;
@@ -4982,7 +4983,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4995,6 +4995,9 @@
 #include "clang/Basic/OpenCLImageTypes.def"
 
 return true;
+
+  case tok::kw_private:
+return getLangOpts().OpenCL;
   }
 }
 
@@ -5196,6 +5199,9 @@
 
 // OpenCL qualifiers:
 case tok::kw_private:
+  if (!getLangOpts().OpenCL)
+goto DoneWithTypeQuals;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:


Index: test/SemaOpenCLCXX/private-access-specifier.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/private-access-specifier.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -pedantic -verify -fsyntax-only
+
+struct B {
+virtual ~B() // expected-error{{expected ';' at end of declaration list}}
+private:
+void foo();
+};
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1410,8 +1410,13 @@
 // cv-qualifier
   case tok::kw_const:
   case tok::kw_volatile:
+return TPResult::True;
+
 // OpenCL address space qualifiers
   case tok::kw_private:
+if (!getLangOpts().OpenCL)
+  return TPResult::False;
+LLVM_FALLTHROUGH;
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4791,7 +4791,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4800,9 +4799,11 @@
   case tok::kw___read_only:
   case tok::kw___read_write:
   case tok::kw___write_only:
-
 return true;
 
+  case tok::kw_private:
+return getLangOpts().OpenCL;
+
   // C11 _Atomic
   case tok::kw__Atomic:
 return true;
@@ -4982,7 +4983,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4995,6 +4995,9 @@
 #include "clang/Basic/OpenCLImageTypes.def"
 
 return true;
+
+  case tok::kw_private:
+return getLangOpts().OpenCL;
   }
 }
 
@@ -5196,6 +5199,9 @@
 
 // OpenCL qualifiers:
 case tok::kw_private:
+  if (!getLangOpts().OpenCL)
+goto DoneWithTypeQuals;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

There is no test here.
Tests should probably go into `clang/test/CodeGen`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

2019-03-27 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a comment.

In D59603#1443651 , @jsji wrote:

> Looks like this may cause some unexpected failures. See 
> https://bugs.llvm.org/show_bug.cgi?id=41247 for more details.


Thanks! I have created a review for the fix: https://reviews.llvm.org/D59874


Repository:
  rC Clang

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

https://reviews.llvm.org/D59603



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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits

danzimm added a comment.

@lebedev.ri right, sorry about that- I prematurely diff'd (got a few terminals 
crossed and thought I was finished with the test already). I will be amending 
with a test and a few other test fixes shortly. Sorry about the 
miscommunication :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

2019-03-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a reviewer: alexfh.
aaron.ballman added a subscriber: alexfh.
aaron.ballman added a comment.

> Intended as my first commit to the llvm-project.

Welcome! Thank you for working on this!

In D59859#1444176 , @lebedev.ri wrote:

> Please always upload all patches with full context (`-U9`)
>  I'm not sure why we want this? What is wrong with simply applying clang-tidy 
> twice?

It doubles the execution time of checking a large project (which may be 
unacceptably slow), and there's no guarantee that twice will be enough in the 
general case (one set of fixes may trigger a different check's diagnostics, 
which may get fixed and trigger another check's diagnostics, etc).

However, it seems like we can run into this sort of interaction in many 
different checks and in many different ways, and I am not certain that we 
should try to tackle the maintenance burden of dealing with those interactions 
ad hoc like this. That said, I'm not certain of a more principled solution 
either. Adding @alexfh to see if he has put any thought into this area and has 
ideas.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59859

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

[PATCH] D59817: [clangd] Add activate command to the vscode extension.

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59817



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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits

danzimm updated this revision to Diff 192436.
danzimm added a comment.

I forgot to add the test originally, this update contains a test and updates to 
old tests to make them pass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873

Files:
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
  clang/test/CodeGenObjC/strong-in-c-struct.m

Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,12 +89,12 @@
 // CHECK: define void @test_constructor_destructor_StrongOuter()
 // CHECK: %[[T:.*]] = alloca %[[STRUCT_STRONGOUTER:.*]], align 8
 // CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V0]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V0]])
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__default_constructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -117,7 +117,7 @@
 // CHECK: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__destructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -149,12 +149,12 @@
 // CHECK: %[[V0:.*]] = load %[[STRUCT_STRONGOUTER]]*, %[[STRUCT_STRONGOUTER]]** %[[S_ADDR]], align 8
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
 // CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[V0]] to i8**
-// CHECK: call void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: call void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[V1]], i8** %[[V2]])
 // CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V3]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V3]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -208,7 +208,7 @@
   StrongOuter t = *s;
 }
 
-/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+/// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
@@ -231,15 +231,15 @@
 // CHECK: define void @test_move_constructor_StrongOuter()
 // CHECK: %[[T1:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T:.*]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T1]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: %[[T2:.*]] = getelementptr inbounds %[[STRUCT_BLOCK_BYREF_T]], %[[STRUCT_BLOCK_BYREF_T]]* %{{.*}}, i32 0, i32 7
 // CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T2]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V9]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V9]])
 
 // CHECK: define internal void @__Block_byref_object_copy_(i8*, i8*)
-// CHECK: call void @__move_constructor_8_8_t0w16_s16_s24_t32w8(
+// CHECK: call void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(
 
-// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// CHECK: define linkonce_odr hidden void @__move_constructor_8_8_S_t0w16_s16_s24_t32w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: %[[SRC_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_AD

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Dan Zimmerman via Phabricator via cfe-commits

danzimm added a comment.

@smeenai please feel free add any reviewers that I might've missed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-27 Thread Takuto Ikuta via Phabricator via cfe-commits

takuto.ikuta added inline comments.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

anton-afanasyev wrote:
> takuto.ikuta wrote:
> > Remove StringRef?
> I use `StringRef("")` to explicitly cast to one of overloaded 
> `TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, 
> fuction_ref(std::string()))`.
I think function_ref(std::string()) does not have constructor receiving 
StringRef, so I feel explicit cast is redundant.




Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+*OS << "{ \"traceEvents\": [\n";
+

anton-afanasyev wrote:
> takuto.ikuta wrote:
> > Don't we want to use json utility for this?
> > https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
> > 
> This implementation looks compact and fast. I've read proposal for this 
> library 
> https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4
>  , it's recent, so I'm not sure it's stable and speed optimized. Do you 
> actually can advise it? I can do it in the next refactor commit as well.
Hmm, I think using json library is preferred because we don't need to take care 
the correctness of json format.
Confirming correctness of json format with many escaped literals is bit hard 
and I'm afraid to miss json format error.



Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+  if (isPrint(C)) {
+OS += C;

anton-afanasyev wrote:
> takuto.ikuta wrote:
> > include StringExtras.h for this?
> Should one do it? It's already implicitly included.
I think it is better to do, because if someone removes the StringExtras.h 
include used for this file, they need to include header in this file at the 
same time. That may require to touch unrelated files in their change.



Comment at: llvm/lib/Support/TimeProfiler.cpp:75
+  void end() {
+assert(!Stack.empty() && "Must call Begin first");
+auto &E = Stack.back();

s/Begin/begin/


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

https://reviews.llvm.org/D58675



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

[PATCH] D59879: [ARM][CMSE] Add commandline option and feature macro

2019-03-27 Thread Javed Absar via Phabricator via cfe-commits

javed.absar created this revision.
javed.absar added a reviewer: dmgreen.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.

This is first in series of patches following the RFC 
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061834.html informing the 
community about providing CMSE support in clang/llvm.

This is patch C1 (as mentioned in the RFC).

It defines macro __ARM_FEATURE_CMSE to 1 for v8-M targets and introduces -mcmse 
option which for v8-M targets sets __ARM_FEATURE_CMSE to 3.   A diagnostic is 
produced when the option is given on architectures without support for Security 
Extensions.


Repository:
  rC Clang

https://reviews.llvm.org/D59879

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -198,6 +198,7 @@
 // V8M_BASELINE: #define __ARM_ARCH_ISA_THUMB 1
 // V8M_BASELINE: #define __ARM_ARCH_PROFILE 'M'
 // V8M_BASELINE-NOT: __ARM_FEATURE_CRC32
+// V8M_BASELINE: #define __ARM_FEATURE_CMSE 1
 // V8M_BASELINE-NOT: __ARM_FEATURE_DSP
 // V8M_BASELINE-NOT: __ARM_FP 0x{{.*}}
 // V8M_BASELINE-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
@@ -210,6 +211,7 @@
 // V8M_MAINLINE: #define __ARM_ARCH_ISA_THUMB 2
 // V8M_MAINLINE: #define __ARM_ARCH_PROFILE 'M'
 // V8M_MAINLINE-NOT: __ARM_FEATURE_CRC32
+// V8M_MAINLINE: #define __ARM_FEATURE_CMSE 1
 // V8M_MAINLINE-NOT: __ARM_FEATURE_DSP
 // V8M_MAINLINE-NOT: #define __ARM_FP 0x
 // V8M_MAINLINE: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
@@ -675,6 +677,24 @@
 // M7-THUMB-ALLOW-FP-INSTR:#define __ARM_FP 0xe
 // M7-THUMB-ALLOW-FP-INSTR:#define __ARM_FPV5__ 1
 
+// Check that -mcmse (security extension) option works correctly for v8-M targets
+// RUN: %clang -target armv8m.base-none-linux-gnu -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target armv8m.main-none-linux-gnu -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target arm-none-linux-gnu -mcpu=cortex-m33 -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target arm -mcpu=cortex-m23 -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// V8M_CMSE-NOT: __ARM_FEATURE_CMSE 1
+// V8M_CMSE: #define __ARM_FEATURE_CMSE 3
+
+// Check that CMSE is not defined on architectures w/o support for security extension
+// RUN: %clang -target arm-arm-none-gnueabi -mcpu=cortex-a5 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// RUN: %clang -target armv8a-none-linux-gnu -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// NOTV8M_CMSE-NOT: __ARM_FEATURE_CMSE
+
+// Check that -mcmse option gives error on non v8-M targets
+// RUN: not %clang -target arm-arm-none-eabi -mthumb -mcmse -mcpu=cortex-m7 -x c -E -dM %s -o - 2>&1 | FileCheck -match-full-lines --check-prefix=NOTV8MCMSE_OPT %s
+// NOTV8MCMSE_OPT: error: -mcmse is not supported for cortex-m7
+
 // Test whether predefines are as expected when targeting v8m cores
 // RUN: %clang -target arm -mcpu=cortex-m23 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=M23 %s
 // M23: #define __ARM_ARCH 8
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2702,6 +2702,7 @@
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
 | Opts.NativeHalfArgsAndReturns;
   Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+  Opts.Cmse = Args.hasArg(OPT_mcmse); // CMSE
 
   // __declspec is enabled by default for the PS4 by the driver, and also
   // enabled for Microsoft Extensions or Borland Extensions, here.
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1386,6 +1386,10 @@
   if (!Args.hasFlag(options::OPT_mimplicit_float,
 options::OPT_mno_implicit_float, true))
 CmdArgs.push_back("-no-implicit-float");
+
+  if (Args.getLastArg(options::OPT_mcmse)) {
+CmdArgs.push_back("-mcmse");
+  }
 }
 
 void Clang::RenderTargetOptions(const llvm::Triple &EffectiveTriple,
Index: lib/Driver/ToolChains/Arch/ARM.cpp

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

2019-03-27 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

We normally add something to the documentation about the checker and/or the 
release notes to say what had changed




Comment at: 
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp:48
   case CK_IntegralToBoolean:
-return Type->isUnsignedIntegerType() ? "0u" : "0";
+if (UppercaseSuffix) {
+  return Type->isUnsignedIntegerType() ? "0U" : "0";

LLVM normally doesn't put braces on small lines



Comment at: 
clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp:221
+return BoolLiteral->getValue() ? "1.0F" : "0.0F";
+  } else {
+return BoolLiteral->getValue() ? "1.0f" : "0.0f";

your don't really need the else here because you return above it


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59859



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

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

2019-03-27 Thread Kristof Beyls via Phabricator via cfe-commits

kristof.beyls added a comment.

Thanks for picking this up, Zola!

I quickly looked through the patch - comparing it with what I had done under 
D49070  and D49073 
.
Apart from the point remarks inline, I had the following immediate thoughts:

1. Could you clang-format the patch?
2. Could you rebase the patch to top-of-trunk (it seems it is a bit behind ToT)?
3. For discussions, seeing the whole patch as it is might be helpful. OTOH, I 
think it also makes reviewing easier if the target-dependent and the 
target-independent parts would be split. I think that could also help others in 
implementing the intrinsics for their targets: they'd have guidance on what 
might be needed from that target-dependent implementation patches for X86 and 
AArch64.
4. Lowering to LFENCE seems a correct lowering to me, but someone more 
knowledgeable about x86 should confirm.
5. I think the LLVM-IR intrinsic should be target-independent, and not 
x86-specific. That would result in less duplication of code when implementing 
support for multiple architectures. I seem to remember that's how I implemented 
this in D49070 . I didn't look so far at the 
SelectionDAG parts of this patch, as I think the differences between my 
implementation in D49070  and this patch may 
go away after making the intrinsic target-independent.

If we'd take the discussion about adding support for intrinsic `T 
__builtin_speculation_safe_value(T v)` further here, I'd be happy to abandon 
the patch series at D49073 .
However, in that case, I think the explanation of the intrinsic there should be 
copied over here to provide a bit more context.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13
 
+#include 
"/usr/local/google/home/zbrid/repos/llvm-project/clang/lib/CodeGen/CodeGenTypeCache.h"
 #include "CGCXXABI.h"

This line doesn't seem to be needed?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3947
+llvm::Type *T = ConvertType(E->getType());
+assert((isa(T) || isa(T)) && 
"unsupported type");
+

line too long - run clang-format?



Comment at: clang/lib/Sema/SemaChecking.cpp:1496
+  case Builtin::BI__builtin_speculation_safe_value:
+   return SemaBuiltinSpeculationSafeValueOverloaded(TheCallResult);
   }

needs one more space of indentation?



Comment at: clang/lib/Sema/SemaChecking.cpp:5325
+  // Too many args
+  if (TheCall->getNumArgs() < 1)
+return Diag(TheCall->getEndLoc(), 
diag::err_typecheck_call_too_many_args_at_most)

Should this be "TheCall->getNumArgs() > 1" (larger than rather then less than)?



Comment at: clang/test/CodeGen/builtin-speculation-safe-value.c:1-2
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck 
-check-prefix=CHECK-SUPPORTED %s
+

When I wrote this test in D49073 this line read "REQUIRES: 
aarch64-registered-target". Looking at this now, I wonder why the requires 
might be needed, beyond the RUN line containing "-triple x86_64-linux-gnu". 
It'd be nice if this test didn't need a REQUIRES line But maybe there is a 
good reason it does need a requires line after all?



Comment at: clang/test/Preprocessor/init.c:9215
 // WEBASSEMBLY-NEXT:#define __GXX_ABI_VERSION 1002
+// WEBASSEMBLY-NEXT:#define __HAVE_SPECULATION_SAFE_VALUE 1
 // WEBASSEMBLY32-NEXT:#define __ILP32__ 1

It seems this is the only intended change in this file; all the other changes 
in this file were unintended for this patch?




Comment at: llvm/include/llvm/IR/Intrinsics.td:1171
  [IntrNoMem, Returned<0>]>;
+
 
//===--===//

accidental new line diff?



Comment at: llvm/include/llvm/IR/IntrinsicsX86.td:4819-4822
+
+//===- Intrinsics to mitigate against miss-speculation exploits 
---===//
+
+def int_speculationsafevalue : Intrinsic<[llvm_any_ty], [LLVMMatchType<0>], 
[]>;

I think this needs to be a target independent LLVM IR intrinsic, not x86 
specific. See D49070. This will also need documentation in LangRef.rst then, 
also see D49070 for a possible documentation I proposed for this intrinsic 
there.



Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:614-628
+  if (Opcode == X86::SpeculationSafeValue32) {
+BuildMI(MBB, NMBBI, DebugLoc(), TII->get(X86::LFENCE));
+++NumInstsInserted;
+++NumLFENCEsInserted;
+MRI->replaceRegWith(MI.getOperand(0).getReg(), 
MI.getOperand(1).getReg());
+MI.eraseFromParent();
+Modified = true;

The lowering of the intrinsic on a 32

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

2019-03-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D59467#1443173 , @Tyker wrote:

> @lebedev.ri where are tests for AST serialization are located ? i didn't find 
> them.

They live in clang\tests\PCH.

In D59467#1440608 , @Tyker wrote:

> i implemented the semantic the changes for if for, while and do while 
> statement and the AST change to if. can you review it and tell me if it is 
> fine so i implement the rest. i didn't update the test so they will fail.

I think this may be moving closer to the correct direction now, but I'm still 
curious to hear if @rsmith has similar opinions.

Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def warn_no_associated_branch : Warning<
+  "attribute %0 not assiciated to any branch is ignored">, 
InGroup;
+def warn_conflicting_attribute : Warning<

Typo: associated

Should probably read "attribute %0 is not associated with a branch and is 
ignored"

Also, I'd rename the diagnostic to be 
`warn_no_likelihood_attr_associated_branch`

Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8165-8166
+  "attribute %0 not assiciated to any branch is ignored">, 
InGroup;
+def warn_conflicting_attribute : Warning<
+  "%0 contradicing with previous %1 attribute">, InGroup;
+

I'd rename the diagnostic to `warn_conflicting_likelihood_attrs` and reword the 
diagnostic to "attribute %0 conflicts with previous attribute %1"

Comment at: clang/include/clang/Sema/Scope.h:163

+  /// BranchAttr - Likelihood attribute associated with this Branch or nullptr
+  Attr *BranchAttr;

Missing full stop at the end of the comment. You should check all the comments 
in the patch to be sure they are full sentences (start with a capital letter, 
end with punctuation, are grammatically correct, etc).

Comment at: clang/include/clang/Sema/Scope.h:164
+  /// BranchAttr - Likelihood attribute associated with this Branch or nullptr
+  Attr *BranchAttr;
+

Rather than store an `Attr` here, would it make sense to instead store a 
`LikelihoodAttr` directly? That may clean up some casts in the code.

Comment at: clang/include/clang/Sema/Sema.h:3845
+  /// emit diagnostics and determinate the hint for and If statement
+  BranchHint HandleIfStmtHint(Stmt *thenStmt, Stmt *elseStmt, Attr *ThenAttr,
+  Attr *ElseAttr);

You should check all of the identifiers introduced by the patch to ensure they 
meet our coding style requirements 
(https://llvm.org/docs/CodingStandards.html). `thenStmt` -> `ThenStmt`, etc.

Comment at: clang/lib/Sema/SemaStmt.cpp:526
+  Attr *ThenAttr, Attr *ElseAttr) {
+  // diagnose branch with attribute and null statement as empty body
+  if (thenStmt && isa(thenStmt) &&

The condition here is incorrect -- it's not checking what kind of attributed 
statement is present to see if it's a likelihood statement.

However, what is the value in diagnosing this construct? It seems 
not-entirely-unreasonable for a user to write, for instance:
```
if (foo) {
  [[likely]];
  ...
} else [[unlikely]];
```

Comment at: clang/lib/Sema/SemaStmt.cpp:548
+} else {
+  if (ThenAttr->getSpelling()[0] == 'l')
+Hint = Taken;

This is fragile -- you should do something more like:
```
if (const auto *LA = dyn_cast(ThenAttr)) {
  Hint = LA->isLikely() ? ... : ...;
}
```
You'll want to add an accessor onto the LikelihoodAttr object in Attr.td to 
implement the `isLikely()` (and `isUnlikely()`) functions based on the 
attribute spelling. There are examples of this in Attr.td.

Comment at: clang/lib/Sema/SemaStmtAttr.cpp:66-73
+if (!ControlScope ||
+!(ControlScope->getFlags() & Scope::ControlScope ||
+  ControlScope->getFlags() & Scope::BreakScope) ||
+ControlScope->getFlags() & Scope::SEHExceptScope ||
+ControlScope->getFlags() & Scope::SEHTryScope ||
+ControlScope->getFlags() & Scope::TryScope ||
+ControlScope->getFlags() & Scope::FnTryCatchScope) {

This doesn't seem to account for `switch` statements, like this, does it?
```
switch (foo) {
[[likely]] case 1: break;
[[unlikely]] case 2: break;
[[likely]] case 3: break;
[[unlikely]] default: break;
}
```
Note, it's perfectly reasonable for there to be repeated attributes here 
because some cases may be more likely than others.

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

https://reviews.llvm.org/D59467

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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:122
 
+- The :bugprone-too-small-loop-variable
+  ` now supports

:doc prefix and ` is missing. Please also rebase patch from trunk.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59870



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

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

2019-03-27 Thread pierre gousseau via Phabricator via cfe-commits

pgousseau updated this revision to Diff 192447.
pgousseau added a comment.

Update patch to reflect changes to the llvm side of the patch.


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

https://reviews.llvm.org/D59221

Files:
  include/clang/Basic/Sanitizers.def
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CrossWindows.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/PS4CPU.cpp
  lib/Driver/ToolChains/Solaris.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -828,3 +828,14 @@
 // CHECK-HWASAN-INTERCEPTOR-ABI: "-default-function-attr" "hwasan-abi=interceptor"
 // CHECK-HWASAN-PLATFORM-ABI: "-default-function-attr" "hwasan-abi=platform"
 // CHECK-HWASAN-FOO-ABI: error: invalid value 'foo' in '-fsanitize-hwaddress-abi=foo'
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address,pointer-compare,pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-ALL
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-compare %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-CMP-NEEDS-ADDRESS
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-SUB-NEEDS-ADDRESS
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract -fno-sanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-SUB
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-compare -fno-sanitize=pointer-compare %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-CMP
+// CHECK-POINTER-ALL: "-cc1{{.*}}-fsanitize={{.*}}pointer-compare,pointer-subtract{{.*}}" {{.*}} "-mllvm" "-asan-detect-invalid-pointer-cmp" "-mllvm" "-asan-detect-invalid-pointer-sub"
+// CHECK-POINTER-CMP-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-compare' only allowed with '-fsanitize=address'
+// CHECK-POINTER-SUB-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-subtract' only allowed with '-fsanitize=address'
+// CHECK-NO-POINTER-SUB-NOT: "{{.*}}asan-detect-invalid-pointer{{.*}}"
+// CHECK-NO-POINTER-CMP-NOT: "{{.*}}asan-detect-invalid-pointer{{.*}}"
Index: lib/Driver/ToolChains/Solaris.cpp
===
--- lib/Driver/ToolChains/Solaris.cpp
+++ lib/Driver/ToolChains/Solaris.cpp
@@ -199,6 +199,8 @@
   // FIXME: Omit X86_64 until 64-bit support is figured out.
   if (IsX86) {
 Res |= SanitizerKind::Address;
+Res |= SanitizerKind::PointerCompare;
+Res |= SanitizerKind::PointerSubtract;
   }
   Res |= SanitizerKind::Vptr;
   return Res;
Index: lib/Driver/ToolChains/PS4CPU.cpp
===
--- lib/Driver/ToolChains/PS4CPU.cpp
+++ lib/Driver/ToolChains/PS4CPU.cpp
@@ -425,6 +425,8 @@
 SanitizerMask toolchains::PS4CPU::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Vptr;
   return Res;
 }
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -463,6 +463,8 @@
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   if (IsX86 || IsX86_64) {
 Res |= SanitizerKind::Address;
+Res |= SanitizerKind::PointerCompare;
+Res |= SanitizerKind::PointerSubtract;
 Res |= SanitizerKind::Function;
 Res |= SanitizerKind::Leak;
 Res |= SanitizerKind::SafeStack;
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -459,6 +459,8 @@
 SanitizerMask toolchains::MinGW::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   return Res;
 }
 
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1317,6 +1317,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCa

[PATCH] D59185: [PowerPC] Set the default PLT mode on musl to Secure PLT

2019-03-27 Thread Justin Hibbits via Phabricator via cfe-commits

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

Looks fine to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59185



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

2019-03-27 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!


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

https://reviews.llvm.org/D59845



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

[clang-tools-extra] r357075 - [clangd] Add activate command to the vscode extension.

2019-03-27 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Wed Mar 27 08:41:59 2019
New Revision: 357075

URL: http://llvm.org/viewvc/llvm-project?rev=357075&view=rev
Log:
[clangd] Add activate command to the vscode extension.

Summary:
This would help minizime the annoying part of not activating the extension
for .cu file.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=357075&r1=357074&r2=357075&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Wed Mar 
27 08:41:59 2019
@@ -24,7 +24,8 @@
 "onLanguage:c",
 "onLanguage:cpp",
 "onLanguage:objective-c",
-"onLanguage:objective-cpp"
+"onLanguage:objective-cpp",
+"onCommand:clangd-vscode.activate"
 ],
 "main": "./out/src/extension",
 "scripts": {
@@ -81,6 +82,10 @@
 {
 "command": "clangd-vscode.switchheadersource",
 "title": "Switch between Source/Header"
+},
+{
+"command": "clangd-vscode.activate",
+"title": "Manually activate clangd extension"
 }
 ],
 "keybindings": [

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=357075&r1=357074&r2=357075&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Wed 
Mar 27 08:41:59 2019
@@ -139,4 +139,8 @@ export function activate(context: vscode
 status.clear();
 }
 })
+// An empty place holder for the activate command, otherwise we'll get an
+// "command is not registered" error.
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.activate', async () => {}));
 }


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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

2019-03-27 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

This probably needs tests if we're lifting it into `llvm`. Sorry there aren't 
any today :-(

BTW the other similar lib in clang I'm aware of is 
https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/tools/clang-refactor/TestSupport.h
It makes different tradeoffs.

We talked offline about the design choices (particularly the markings used) 
that make annotations terse but limit the applicability, e.g. code containing 
`^` cannot be annotated. (I'd say the limitations have been OK for clangd). 
Possible changes:

- add an escaping mechanism so that currently impossible literal code can be 
encoded. e.g. `^^` is a literal `^`, or `$^` is a literal `^` or similar. I'd 
be fine with almost any option here.
- allow users of the library to customize the markings used (e.g. pass in an 
`Annotations::Sigils` struct, and provide defaults and possibly an alternate 
set)
- change the markings to make them more verbose, e.g. `$^` or `#^#` instead of 
`^`. Verbosity is clearly a question of taste here, I'm not particularly in 
favor of such a change. Choice of markings is a complicated and constrained 
problem.
- change the markings to make them less ambiguous without being more verbose, 
e.g. `$` instead of `^` and `|[foo]|` instead of `[[foo]]`. I'm more amenable 
to this, though choice of markings remains important and hard.
- add a `FIXME` promising to do any of these things in the future. This option 
is certainly fine from clangd's perspective. This probably forces future 
changes to be backwards compatible or have broad consensus.




Comment at: clang-tools-extra/unittests/clangd/Annotations.h:8
 
//===--===//
-//
-// Annotations lets you mark points and ranges inside source code, for tests:
-//
-//Annotations Example(R"cpp(
-//   int complete() { x.pri^ }  // ^ indicates a point
-//   void err() { [["hello" == 42]]; }  // [[this is a range]]
-//   $definition^class Foo{};   // points can be named: 
"definition"
-//   $fail[[static_assert(false, "")]]  // ranges can be named too: "fail"
-//)cpp");
-//
-//StringRef Code = Example.code();  // annotations stripped.
-//std::vector PP = Example.points();  // all unnamed points
-//Position P = Example.point(); // there must be exactly 
one
-//Range R = Example.range("fail");  // find named ranges
-//
-// Points/ranges are coordinates into `code()` which is stripped of 
annotations.
-//
-// Ranges may be nested (and points can be inside ranges), but there's no way
-// to define general overlapping ranges.
-//
+// A clangd-specific version of llvm/Testing/Support/Annotations.h, replaces
+// offsets and offset-based ranges with types from the LSP protocol.

The choice to shadow the base methods is interesting. I guess we can always use 
llvm::Annotations if we want byte offsets (I know there are some), not sure if 
we ever want a mix.
Anyway this is nice and clean, and minimizes the diff. We can change later if 
we care.



Comment at: llvm/include/llvm/Testing/Support/Annotations.h:42
+/// represents a half-open range.
+struct Range {
+  size_t Begin = 0;

ilya-biryukov wrote:
> This name is probably too generic, happy to change the name or implement this 
> in an alternative manner.
I'd probably suggest making this `Annotations::Range`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

2019-03-27 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!


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

https://reviews.llvm.org/D59665



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

[PATCH] D59817: [clangd] Add activate command to the vscode extension.

2019-03-27 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357075: [clangd] Add activate command to the vscode 
extension. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59817

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -24,7 +24,8 @@
 "onLanguage:c",
 "onLanguage:cpp",
 "onLanguage:objective-c",
-"onLanguage:objective-cpp"
+"onLanguage:objective-cpp",
+"onCommand:clangd-vscode.activate"
 ],
 "main": "./out/src/extension",
 "scripts": {
@@ -81,6 +82,10 @@
 {
 "command": "clangd-vscode.switchheadersource",
 "title": "Switch between Source/Header"
+},
+{
+"command": "clangd-vscode.activate",
+"title": "Manually activate clangd extension"
 }
 ],
 "keybindings": [
Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -139,4 +139,8 @@
 status.clear();
 }
 })
+// An empty place holder for the activate command, otherwise we'll get an
+// "command is not registered" error.
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.activate', async () => {}));
 }


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -24,7 +24,8 @@
 "onLanguage:c",
 "onLanguage:cpp",
 "onLanguage:objective-c",
-"onLanguage:objective-cpp"
+"onLanguage:objective-cpp",
+"onCommand:clangd-vscode.activate"
 ],
 "main": "./out/src/extension",
 "scripts": {
@@ -81,6 +82,10 @@
 {
 "command": "clangd-vscode.switchheadersource",
 "title": "Switch between Source/Header"
+},
+{
+"command": "clangd-vscode.activate",
+"title": "Manually activate clangd extension"
 }
 ],
 "keybindings": [
Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -139,4 +139,8 @@
 status.clear();
 }
 })
+// An empty place holder for the activate command, otherwise we'll get an
+// "command is not registered" error.
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.activate', async () => {}));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r357078 - [clangd] Fix the inconsistent code indent in vscode extension, NFC.

2019-03-27 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Wed Mar 27 08:50:33 2019
New Revision: 357078

URL: http://llvm.org/viewvc/llvm-project?rev=357078&view=rev
Log:
[clangd] Fix the inconsistent code indent in vscode extension, NFC.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=357078&r1=357077&r2=357078&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Wed 
Mar 27 08:50:33 2019
@@ -102,27 +102,27 @@ export function activate(context: vscode
 revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
 };
 
-  const clangdClient = new vscodelc.LanguageClient('Clang Language Server', 
serverOptions, clientOptions);
-  console.log('Clang Language Server is now active!');
-  context.subscriptions.push(clangdClient.start());
-  context.subscriptions.push(vscode.commands.registerCommand(
-  'clangd-vscode.switchheadersource', async () => {
-const uri =
-vscode.Uri.file(vscode.window.activeTextEditor.document.fileName);
-if (!uri) {
-  return;
-}
-const docIdentifier =
-vscodelc.TextDocumentIdentifier.create(uri.toString());
-const sourceUri = await clangdClient.sendRequest(
-SwitchSourceHeaderRequest.type, docIdentifier);
-if (!sourceUri) {
-  return;
-}
-const doc = await vscode.workspace.openTextDocument(
-vscode.Uri.parse(sourceUri));
-vscode.window.showTextDocument(doc);
-  }));
+const clangdClient = new vscodelc.LanguageClient('Clang Language 
Server',serverOptions, clientOptions);
+console.log('Clang Language Server is now active!');
+context.subscriptions.push(clangdClient.start());
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.switchheadersource', async () => {
+const uri =
+
vscode.Uri.file(vscode.window.activeTextEditor.document.fileName);
+if (!uri) {
+return;
+}
+const docIdentifier =
+vscodelc.TextDocumentIdentifier.create(uri.toString());
+const sourceUri = await clangdClient.sendRequest(
+SwitchSourceHeaderRequest.type, docIdentifier);
+if (!sourceUri) {
+return;
+}
+const doc = await vscode.workspace.openTextDocument(
+vscode.Uri.parse(sourceUri));
+vscode.window.showTextDocument(doc);
+}));
 const status = new FileStatus();
 context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(() => 
{
 status.updateStatus();


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

[clang-tools-extra] r357082 - [clangd] Bump vscode-clangd v0.0.12.

2019-03-27 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Wed Mar 27 09:01:25 2019
New Revision: 357082

URL: http://llvm.org/viewvc/llvm-project?rev=357082&view=rev
Log:
[clangd] Bump vscode-clangd v0.0.12.

CHANGELOG:
- add an explicit command to activate the extension.
- support .cu files (the extension is not activated for .cu files by default,
  you need to manually activate the extension).

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=357082&r1=357081&r2=357082&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Wed Mar 
27 09:01:25 2019
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.11",
+"version": "0.0.12",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added a project: clang.

By adding a hook to consume all tokens produced by the preprocessor.
The intention of this change is to make it possible to consume the
expanded tokens without re-runnig the preprocessor with minimal changes
to the preprocessor and minimal performance penalty when preprocessing
without recording the tokens.

The added hook is very low-level and reconstructing the expanded token
stream requires more work in the client code, the actual algorithm to
collect the tokens using this hook can be found in the follow-up change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59885

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -864,6 +864,7 @@
 void Preprocessor::Lex(Token &Result) {
   // We loop here until a lex function returns a token; this avoids recursion.
   bool ReturnedToken;
+  bool Cached = false;
   do {
 switch (CurLexerKind) {
 case CLK_Lexer:
@@ -875,6 +876,7 @@
 case CLK_CachingLexer:
   CachingLex(Result);
   ReturnedToken = true;
+  Cached = true;
   break;
 case CLK_LexAfterModuleImport:
   LexAfterModuleImport(Result);
@@ -893,6 +895,8 @@
   }
 
   LastTokenWasAt = Result.is(tok::at);
+  if (OnToken && ReturnedToken && !Cached)
+OnToken(Result);
 }
 
 /// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -33,6 +33,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -48,8 +49,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -124,6 +125,7 @@
   friend class VAOptDefinitionContext;
   friend class VariadicMacroScopeGuard;
 
+  llvm::unique_function OnToken;
   std::shared_ptr PPOpts;
   DiagnosticsEngine*Diags;
   LangOptions   &LangOpts;
@@ -911,6 +913,14 @@
   }
   /// \}
 
+  /// Register a function that would be called on each token seen by the
+  /// preprocessor. This is a very low-level hook, the produced token stream is
+  /// tied to the internals of the preprocessor so interpreting result of the
+  /// callback is hard.
+  void setTokenWatcher(llvm::unique_function F) {
+OnToken = std::move(F);
+  }
+
   bool isMacroDefined(StringRef Id) {
 return isMacroDefined(&Identifiers.get(Id));
   }


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -864,6 +864,7 @@
 void Preprocessor::Lex(Token &Result) {
   // We loop here until a lex function returns a token; this avoids recursion.
   bool ReturnedToken;
+  bool Cached = false;
   do {
 switch (CurLexerKind) {
 case CLK_Lexer:
@@ -875,6 +876,7 @@
 case CLK_CachingLexer:
   CachingLex(Result);
   ReturnedToken = true;
+  Cached = true;
   break;
 case CLK_LexAfterModuleImport:
   LexAfterModuleImport(Result);
@@ -893,6 +895,8 @@
   }
 
   LastTokenWasAt = Result.is(tok::at);
+  if (OnToken && ReturnedToken && !Cached)
+OnToken(Result);
 }
 
 /// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -33,6 +33,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -48,8 +49,8 @@
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -124,6 +125,7 @@
   friend class VAOptDefinitionContext;
   friend class VariadicMacroScopeGuard;
 
+  llvm::unique_function OnToken;
   std::shared_ptr PPOpts;
   DiagnosticsEngine*Diags;
   LangOptions   &LangOpts;
@@ -911,6 +913,14 @@
   }
   /// \}
 
+  /// Register a function that would be called on each token seen by the
+  /// preprocessor. This is a very low-level hook, the produced token stream is
+  /// tied to the internals of the preprocessor so interpreting result of the
+  /// callback is hard.
+  void setTokenWatcher(llvm::unique_function F) {
+OnToken = std::move(F);
+  }
+
   bool isMacroDefined(StringRef Id) {
 r

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr.
Herald added subscribers: jdoerfert, mgorny.
Herald added a project: clang.

TokenBuffer stores the list of tokens for a file obtained after
preprocessing. This is a base building block for syntax trees,
see [1] for the full proposal on syntax trees.

This commits also starts a new sub-library of ClangTooling, which
would be the home for the syntax trees and syntax-tree-based refactoring
utilities.

[1]: https://lists.llvm.org/pipermail/cfe-dev/2019-February/061414.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/TokenBuffer.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/TokenBuffer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokenBufferTest.cpp

Index: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
@@ -0,0 +1,471 @@
+//===- TokenBufferTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/TokenBuffer.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+// Debug printers.
+// FIXME: This should live somewhere else or be implemented as 'operator
+// <<(raw_ostream&, T)'.
+namespace clang {
+namespace tok {
+inline void PrintTo(TokenKind K, std::ostream *OS) {
+  *OS << tok::getTokenName(K);
+}
+} // namespace tok
+namespace syntax {
+inline void PrintTo(const syntax::Token &T, std::ostream *OS) {
+  PrintTo(T.kind(), OS);
+  OS->flush();
+}
+} // namespace syntax
+} // namespace clang
+
+namespace {
+// Matchers for clang::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto &L = std::get<0>(arg);
+  auto &R = std::get<1>(arg);
+  if (L.kind() != R.kind())
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+
+class TokenBufferTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Tokens.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer &Result) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

I'm happy to assess the performance costs of this change if needed (the 
function is obviously on the hot path), but I hope this won't add any 
significant performance costs.
Also happy to consider alternative approaches to collect the tokens when 
preprocessing, the current implementation is complicated (search for 
'TokenCollector' in D59887 ) and my intuition 
is that moving parts of it to the preprocessor should simplify things 
considerably.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-03-27 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

I think in general this is a good direction. What do you think about other 
heuristics?
E.g. one could say that a vector will not contain more then X GB or similar. 
That is probably more complicated to figure out though.




Comment at: docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:44
+long size = 294967296l;
+for (unsigned i = 0; i < size; ++i) {} // no warning
+for (int i = 0; i < size; ++i) {} // warning

no warning for which value? 31? Please make that explicit in the doc.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59870



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

2019-03-27 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:8
+//===--===//
+#include "clang/Tooling/Syntax/TokenBuffer.h"
+#include "clang/Basic/Diagnostic.h"

Please separate with empty line.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:28
+}
+llvm::StringRef syntax::Token::text(const SourceManager &SM) const {
+  bool Invalid = false;

Please separate with empty line.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:323
+  PP.getLangOpts(), Tokens);
+  auto *CB = CBOwner.get();
+

Return type is not obvious. Actually variable is used only one, so it's not 
necessary.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:133
+  FileName};
+auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+assert(CI);

Return type is not obvious, so auto should not be used.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:157
+std::string NullTerminated = Text.str();
+auto FID = SourceMgr->createFileID(llvm::MemoryBuffer::getMemBufferCopy(
+StringRef(NullTerminated.data(), NullTerminated.size() + 1)));

Return type is not obvious, so auto should not be used.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:165
+  void checkTokens(llvm::StringRef ExpectedText) {
+auto TokenizedCode = tokenize(ExpectedText);
+std::vector ExpectedTokens = TokenizedCode.tokens();

Return type is not obvious, so auto should not be used.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:186
+  void checkExpansions(llvm::ArrayRef Expected) {
+auto Actual = Buffer.expansions();
+ASSERT_EQ(Actual.size(), Expected.size());

Return type is not obvious, so auto should not be used.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:190
+for (unsigned I = 0; I < Actual.size(); ++I) {
+  auto &A = Actual[I];
+  auto &E = Expected[I];

Return type is not obvious, so auto should not be used.



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:191
+  auto &A = Actual[I];
+  auto &E = Expected[I];
+

Return type is not obvious, so auto should not be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

2019-03-27 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D58797#1443856 , @erik.pilkington 
wrote:

> I added it in r357041.


LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

2019-03-27 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

In D59485#1439628 , @martong wrote:

> In D59485#1439570 , @martong wrote:
>
> > In D59485#1439390 , @teemperor 
> > wrote:
> >
> > > > Well, I still don't understand how LLDB synthesis the AST. 
> > > >  So you have a C++ module in a .pcm file. This means you could create 
> > > > an AST from that with ASTReader from it's .clang_ast section, right? In 
> > > > that case the AST should be complete with all type information. If this 
> > > > was the case then I don't see why it is not possible to use 
> > > > clang::ASTImporter to import templates and specializations, since we do 
> > > > exactly that in CTU.
> > > > 
> > > > Or do you guys create the AST from the debug info, from the .debug_info 
> > > > section of .pcm module file? And this is why AST is incomplete? I've 
> > > > got this from 
> > > > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> > > >  If this is the case, then comes my naiive quiestion: Why don't you use 
> > > > the ASTReader?
> > >
> > > There are no C++ modules involved in our use case beside the generic 
> > > `std` module. No user-written code is read from a module and we don't 
> > > have any PCM file beside the `std` module we build for the expression 
> > > evaluator.
> > >
> > > We use the ASTReader to directly read the `std` module contents into the 
> > > expression evaluation ASTContext, but this doesn't give us the decls for 
> > > the instantiations the user has made (e.g. `std::vector`). We only 
> > > have these user instantiations in the 'normal' debug info where we have a 
> > > rather minimal AST (again, no modules are not involved in building this 
> > > debug info AST). The `InternalImport` in the LLDB patch just stitches the 
> > > module AST and the debug info AST together when we import something that 
> > > we also have (in better quality from the C++ module) in the target 
> > > ASTContext.
> >
> >
> > Thank you for the explanation.
> >  Now I understand you directly create specializations from the std module 
> > and intercept the import to avoid importing broken specializations from the 
> > debug info, this makes sense.
>
>
> Really, just one last question to see the whole picture: If 
> clang::ASTImporter were capable of handling a specialization/instantiation 
> with missing info then we could use that. So the reason for this interception 
> is that clang::ASTImporter::VisitClassTemplateSpecializationDecl cannot 
> handle the specialization it receives because that or its dependent nodes has 
> too many missing data, right?


Feel free to ask! I'm just traveling so my internet access (and my reply rate) 
is a bit low at the moment.

Not sure if we can easily merge the logic of D59537 
 into the ASTImporter. It has no logic at all 
to actually determine if a source AST node has missing data but solely relies 
on our knowledge that our AST in LLDB isn't very useful for std templates. Also 
instantiating a template instead of importing an existing instantiation seems 
like a very rare scenario. I'm not even sure how we would test having a broken 
AST with Clang (the parser won't produce one, so we would have to hack our own 
AST builder for broken nodes).

If there is a reasonable way to get this logic into the ASTImporter I have no 
objection against doing so. The only problem I see is that I can't come up with 
a reasonable way of merging/testing the logic in the ASTImporter (and what 
other application would benefit from it).


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

https://reviews.llvm.org/D59485



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:130
+  /// result.
+  llvm::ArrayRef expansions() const { return Expansions; }
+  /// Tokens of macro directives and top-level macro expansions. These are not

@gribozavr, note that I left the name expansions.
The reasoning is that it seems to describe both the functional and 
non-functional macros, while "invocation" is only applicable to functional 
macros.
```
#define FOO int
#define ID(a)

FOO // a macro expansion
ID(a = 10;); // a macro invocation *and* a macro expansion.
```



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:157
+std::string NullTerminated = Text.str();
+auto FID = SourceMgr->createFileID(llvm::MemoryBuffer::getMemBufferCopy(
+StringRef(NullTerminated.data(), NullTerminated.size() + 1)));

Eugene.Zelenko wrote:
> Return type is not obvious, so auto should not be used.
It is `createFileID` definitely returns `FileID`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

2019-03-27 Thread Amara Emerson via Phabricator via cfe-commits

aemerson added a comment.

Ping. I've filed PR41260 for the code quality issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59615



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus accepted this revision.
Szelethus added a comment.

Amazing work! Thanks!

In D58367#1443783 , @NoQ wrote:

> Remove memoization for now. Address a few inline comments. I'm mostly done 
> with this first patch, did i accidentally miss anything?
>
> In D58367#1402796 , @Szelethus wrote:
>
> > 1. It would be great to have unit tests for this.
>
>
> Mm, i have problems now that checker registry is super locked down: i need a 
> bug report object => i need a checker (or at least a `CheckName`) => i need 
> the whole `AnalysisConsumer` => i can no longer override `ASTConsumer` 
> methods for testing purposes (because `AnalysisConsumer` is a final sub-class 
> of `ASTConsumer`). Do you have a plan B for that?

Saw this, will think about it! Though I'm a little unsure what you mean under 
the checker registry being locked down.

> I also generally don't see many good unit tests we could write here, that 
> would add much to the integration tests we already have, but this memoization 
> problem could have made a nice unit test.

I don't insist, but unlike most of the subprojects within clang, we have 
very-very few infrastructural unit tests. I don't even find them to be that 
important for testing purposes, but more as minimal examples: I remember that 
`unittests/StaticAnalyzer/RegisterCustomChecker.cpp` was a godsent when I 
worked on checker dependencies.

Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:600
+public:
+  typedef std::function
+  Callback;

Prefer using.

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

https://reviews.llvm.org/D58367

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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:323
+  PP.getLangOpts(), Tokens);
+  auto *CB = CBOwner.get();
+

Eugene.Zelenko wrote:
> Return type is not obvious. Actually variable is used only one, so it's not 
> necessary.
- The type is quite obvious from the previous line.
- We need the variable as we `std::move` the `unique_ptr` in the next line and 
sill want to access the pointer later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

2019-03-27 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik updated this revision to Diff 192466.
shafik added a comment.

Fixes based on comments.


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

https://reviews.llvm.org/D59845

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,12 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,12 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

2019-03-27 Thread Shafik Yaghmour via Phabricator via cfe-commits

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



Comment at: lib/AST/ASTImporter.cpp:1951
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+

JDevlieghere wrote:
> ```
> if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum)) 
>   if (auto *ToOriginEnum = dyn_cast(ToOrigin))
> ToEnum = ToOriginEnum;
> ```
I normally just match the local style but this is indeed much better style.


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

https://reviews.llvm.org/D59845



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

2019-03-27 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:323
+  PP.getLangOpts(), Tokens);
+  auto *CB = CBOwner.get();
+

ilya-biryukov wrote:
> Eugene.Zelenko wrote:
> > Return type is not obvious. Actually variable is used only one, so it's not 
> > necessary.
> - The type is quite obvious from the previous line.
> - We need the variable as we `std::move` the `unique_ptr` in the next line 
> and sill want to access the pointer later.
Lines may be separated in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak accepted this revision.
ahatanak 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/D59873/new/

https://reviews.llvm.org/D59873



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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

Do you need someone to commit this for you?




Comment at: clang/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m:1
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  
-fobjc-runtime=ios-11.0 -emit-llvm -o - -DUSESTRUCT -I %S/Inputs %s | FileCheck 
%s
+

I think you can get rid of `-fblocks`, `-fobjc-runtime=ios-11.0`, 
`-DUSESTRUCT`, and `-I %S/Inputs`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D59874: [PR41247] Fixed parsing of private keyword in C++

2019-03-27 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Otherwise LGTM.




Comment at: test/SemaOpenCLCXX/private-access-specifier.cpp:2
+// RUN: %clang_cc1 %s -pedantic -verify -fsyntax-only
+
+struct B {

Please include a comment explaining why this test file is in 
test/SemaOpenCLCXX/ (because it's testing a regression in general code arising 
from an OpenCL C++ feature).


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

https://reviews.llvm.org/D59874



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

[PATCH] D59457: [analyzer][NFC] Use capital variable names, move methods out-of-line, rename some in CheckerRegistry

2019-03-27 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59457



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

[PATCH] D59863: [HIP] Support gpu arch gfx906+sram-ecc

2019-03-27 Thread Artem Belevich via Phabricator via cfe-commits

tra added inline comments.



Comment at: lib/Basic/Cuda.cpp:113
+  case CudaArch::GFX906_SRAM_ECC: // TBA
+return "gfx906+sram-ecc";
   case CudaArch::GFX909: // TBA

Wording nit:
Does it mean `+(SRAM, ECC)` or `+SRAM, -ECC` ?

From the rest of the changes I guess it's the former, but the syntax used can 
easily be interpreted as the later.

Perhaps change the feature name to `sram_ecc` and GPU name to something less 
verbose. `gfx906se` ?


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

https://reviews.llvm.org/D59863



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

r357100 - [ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

2019-03-27 Thread Shafik Yaghmour via cfe-commits

Author: shafik
Date: Wed Mar 27 10:47:36 2019
New Revision: 357100

URL: http://llvm.org/viewvc/llvm-project?rev=357100&view=rev
Log:
[ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent 
re-importing an EnumDecl while trying to complete it

Summary:
We may try and re-import an EnumDecl while trying to complete it in 
IsStructuralMatch(...) specialization for EnumDecl. This change mirrors a 
similar fix for the specialization for RecordDecl.

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

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

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=357100&r1=357099&r2=357100&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Mar 27 10:47:36 2019
@@ -1946,6 +1946,12 @@ bool ASTNodeImporter::IsStructuralMatch(
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

2019-03-27 Thread Shafik Yaghmour via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
shafik marked an inline comment as done.
Closed by commit rL357100: [ASTImporter] Fix IsStructuralMatch specialization 
for EnumDecl to prevent re… (authored by shafik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59845?vs=192466&id=192475#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59845

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,12 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,12 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r357102 - [clangd] Support utf-8 offsets (rather than utf-16) as a protocol extension

2019-03-27 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Wed Mar 27 10:47:49 2019
New Revision: 357102

URL: http://llvm.org/viewvc/llvm-project?rev=357102&view=rev
Log:
[clangd] Support utf-8 offsets (rather than utf-16) as a protocol extension

Summary:
Still some pieces to go here: unit tests for new SourceCode functionality and
a command-line flag to force utf-8 mode. But wanted to get early feedback.

Reviewers: hokein

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

Tags: #clang

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

Added:
clang-tools-extra/trunk/test/clangd/utf8.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/SymbolLocation.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=357102&r1=357101&r2=357102&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Mar 27 10:47:49 2019
@@ -13,6 +13,7 @@
 #include "Trace.h"
 #include "URI.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
@@ -93,6 +94,7 @@ public:
   MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
 
   bool onNotify(llvm::StringRef Method, llvm::json::Value Params) override {
+WithContext HandlerContext(handlerContext());
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
@@ -109,6 +111,7 @@ public:
 
   bool onCall(llvm::StringRef Method, llvm::json::Value Params,
   llvm::json::Value ID) override {
+WithContext HandlerContext(handlerContext());
 // Calls can be canceled by the client. Add cancellation context.
 WithContext WithCancel(cancelableRequestContext(ID));
 trace::Span Tracer(Method);
@@ -129,6 +132,7 @@ public:
 
   bool onReply(llvm::json::Value ID,
llvm::Expected Result) override {
+WithContext HandlerContext(handlerContext());
 // We ignore replies, just log them.
 if (Result)
   log("<-- reply({0})", ID);
@@ -259,6 +263,13 @@ private:
 if (It != RequestCancelers.end())
   It->second.first(); // Invoke the canceler.
   }
+
+  Context handlerContext() const {
+return Context::current().derive(
+kCurrentOffsetEncoding,
+Server.NegotiatedOffsetEncoding.getValueOr(OffsetEncoding::UTF16));
+  }
+
   // We run cancelable requests in a context that does two things:
   //  - allows cancellation using RequestCancelers[ID]
   //  - cleans up the entry in RequestCancelers when it's no longer needed
@@ -302,6 +313,20 @@ void ClangdLSPServer::notify(llvm::Strin
 
 void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+  // Determine character encoding first as it affects constructed ClangdServer.
+  if (Params.capabilities.offsetEncoding && !NegotiatedOffsetEncoding) {
+NegotiatedOffsetEncoding = OffsetEncoding::UTF16; // fallback
+for (OffsetEncoding Supported : *Params.capabilities.offsetEncoding)
+  if (Supported != OffsetEncoding::UnsupportedEncoding) {
+NegotiatedOffsetEncoding = Supported;
+break;
+  }
+  }
+  llvm::Optional WithOffsetEncoding;
+  if (NegotiatedOffsetEncoding)
+WithOffsetEncoding.emplace(kCurrentOffsetEncoding,
+   *NegotiatedOffsetEncoding);
+
   if (Params.rootUri && *Params.rootUri)
 ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
   else if (Params.rootPath && !Params.rootPath->empty())
@@ -331,7 +356,7 @@ void ClangdLSPServer::onInitialize(const
   SupportsHierarchicalDocumentSymbol =
   Params.capabilities.HierarchicalDocumentSymbol;
   SupportFileStatus = Params.initializationOptions.FileStatus;
-  Reply(llvm::json::Object{
+  llvm::json::Object Result{
   {{"capabilities",
 llvm::json::Object{
 {"textDocumentSync", (int)TextDocumentSyncKind::Incremental},
@@ -369,7 +394,10 @@ void ClangdLSPServer::onInitialize(const
ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
  }},
 {"typeHierarchyProvider", true},
-);
+;
+  if (NegotiatedOffsetEncoding)
+Result["offsetEncoding"] = *NegotiatedOffsetEncoding;
+  Reply(std::move(Result));
 }
 
 void ClangdLSPSe

[PATCH] D58275: [clangd] Support utf-8 offsets (rather than utf-16) as a protocol extension

2019-03-27 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rCTE357102: [clangd] Support utf-8 offsets (rather than 
utf-16) as a protocol extension (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58275?vs=187002&id=192477#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58275

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/index/IndexAction.cpp
  clangd/index/SymbolLocation.h
  clangd/tool/ClangdMain.cpp
  test/clangd/utf8.test
  unittests/clangd/SourceCodeTests.cpp

Index: test/clangd/utf8.test
===
--- test/clangd/utf8.test
+++ test/clangd/utf8.test
@@ -0,0 +1,32 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# This test verifies that we can negotiate UTF-8 offsets via protocol extension.
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"offsetEncoding":["utf-8","utf-16"]},"trace":"off"}}
+# CHECK: "offsetEncoding": "utf-8"
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"/*ö*/int x;\nint y=x;"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":1,"character":6}}}
+# /*ö*/int x;
+# 01234567890
+# x is character (and utf-16) range [9,10) but byte range [10,11).
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 11,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 10,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -6,6 +6,8 @@
 //
 //===--===//
 #include "Annotations.h"
+#include "Context.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
@@ -21,14 +23,9 @@
 using llvm::HasValue;
 
 MATCHER_P2(Pos, Line, Col, "") {
-  return arg.line == Line && arg.character == Col;
+  return arg.line == int(Line) && arg.character == int(Col);
 }
 
-// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
-const char File[] = R"(0:0 = 0
-1:0 → 8
-2:0 🡆 18)";
-
 /// A helper to make tests easier to read.
 Position position(int line, int character) {
   Position Pos;
@@ -52,8 +49,28 @@
   EXPECT_EQ(lspLength("¥"), 1UL);
   // astral
   EXPECT_EQ(lspLength("😂"), 2UL);
+
+  WithContextValue UTF8(kCurrentOffsetEncoding, OffsetEncoding::UTF8);
+  EXPECT_EQ(lspLength(""), 0UL);
+  EXPECT_EQ(lspLength("ascii"), 5UL);
+  // BMP
+  EXPECT_EQ(lspLength("↓"), 3UL);
+  EXPECT_EQ(lspLength("¥"), 2UL);
+  // astral
+  EXPECT_EQ(lspLength("😂"), 4UL);
 }
 
+// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
+const char File[] = R"(0:0 = 0
+1:0 → 8
+2:0 🡆 18)";
+struct Line {
+  unsigned Number;
+  unsigned Offset;
+  unsigned Length;
+};
+Line FileLines[] = {Line{0, 0, 7}, Line{1, 8, 9}, Line{2, 18, 11}};
+
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), llvm::Failed());
@@ -113,6 +130,23 @@
   // line out of bounds
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(3, 0)), llvm::Failed());
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(3, 1)), llvm::Failed());
+
+  // Test UTF-8, where transformations are trivial.
+  WithContextValue UTF8(kCurrentOffsetEncoding, OffsetEncoding::UTF8);
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), llvm::Failed());
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(3, 0)), llvm::Failed());
+  for (Line L : FileLines) {
+EXPECT_THAT_EXPECTED(positionToOffset(File, position(L.Number, -1)),
+ llvm::Failed()); // out of range
+for (unsigned I = 0; I <= L.Length; ++I)
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(L.Number, I)),
+   llvm::HasValue(L.Offset + I));
+EXPECT_THAT_EXPECTED(positionToOffset(File, position(L.Number, L.Length+1)),
+

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

2019-03-27 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D59347#1443051 , @dblaikie wrote:

> @asmith: Where's the LLVM-side change/review that goes with this, btw?
>
> In D59347#1442970 , @probinson wrote:
>
> > As a rule I would prefer flags with positive names, as it's slightly easier 
> > to read `!isTrivial` than `!isNonTrivial`. And trivially shorter. :-)
>
>
> Fair enough - I was mostly coming at this from the "the patch that was 
> committed should be reverted" & then we could haggle over other things, but 
> fair point.

Hmm, one other thought: Technically "non trivial" is perhaps more accurate/less 
error prone. Only marking structures as "trivial" but other types without that 
marker makes it more subtle (since not all trivial types would be marked 
trivial - only those of a classification that means they /could/ be 
non-trivial). Whereas marking the non-trivial types is more broadly accurate.

@asmith - is this patch now essentially a revert of the trivial flag addition? 
Or are there any parts that were not reverted, if so, why not? (I would 
expect/imagine all the testing could be reverted too - since the NonTrivial 
flag was presumably already tested appropriately?)

Repository:
  rC Clang

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

https://reviews.llvm.org/D59347

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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

2019-03-27 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

Also, it might be worth adding a third level of struct to the test, to show 
that it handles arbitrary nesting correctly (which it does).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D46140: [coroutines] Add std::experimental::task type

2019-03-27 Thread Casey Carter via Phabricator via cfe-commits

CaseyCarter requested changes to this revision.
CaseyCarter added a subscriber: mclow.lists.
CaseyCarter added inline comments.
This revision now requires changes to proceed.



Comment at: include/experimental/__memory:80
+{
+return (__s + __a - 1) & ~(__a - 1);
+}

This is missing preconditions that `__a` is a power of 2, and that `__s <=  
-__a`.



Comment at: include/experimental/task:141
+
+void* __pointer = __charAllocator.allocate(
+__get_padded_frame_size_with_allocator<_CharAlloc>(__size));

The return value of `allocate` isn't necessarily convertible to `void*`, it 
could be a fancy pointer. We should either `static_assert(is_same_v::void_pointer, void*>, "Piss off with your fancy 
pointers");` or use `pointer_traits` here and in `__deallocFunc` to unfancy and 
re-fancy the pointer.



Comment at: include/experimental/task:210
+private:
+  friend struct __task_promise_final_awaitable;
+

Pure style comment: I recommend using the non-elaborated `friend 
__task_promise_final_awaitable;` whenever possible. `friend struct foo` 
declares or redeclares `struct foo` in the enclosing namespace, whereas `friend 
foo` uses name lookup to find `foo` and makes it a friend. The latter form 
makes it far easier to analyze compiler errors when you screw something up in 
maintenance. (And on 480)



Comment at: include/experimental/task:220
+public:
+  __task_promise() _NOEXCEPT : __state_(_State::__no_value) {}
+

This mem-initializer for `__state_` is redundant with the default member 
initializer on 306.



Comment at: include/experimental/task:227
+  break;
+#ifndef _LIBCPP_NO_EXCEPTIONS
+case _State::__exception:

I suggest moving the `case` label and `break` outside the `#ifndef` here so the 
compiler won't warn about this case being unhandled when 
`_LIBCPP_NO_EXCEPTIONS`.



Comment at: include/experimental/task:244
+exception_ptr(current_exception());
+__state_ = _State::__exception;
+#else

Are you certain that `unhandled_exception` can't possibly be called after 
storing a value? If so, this would leak the value.



Comment at: include/experimental/task:254
+  template ::value, int> = 0>
+  void return_value(_Value&& __value)

Style: you've been using the `_v` variable templates for traits elsewhere.



Comment at: include/experimental/task:261
+  template 
+  auto return_value(std::initializer_list<_Value> __initializer) _NOEXCEPT_(
+  (is_nothrow_constructible_v<_Tp, std::initializer_list<_Value>>))

Unnecessary `std::` qualifier. (Occurs repeatedly.)



Comment at: include/experimental/task:263
+  (is_nothrow_constructible_v<_Tp, std::initializer_list<_Value>>))
+  -> std::enable_if_t<
+  std::is_constructible_v<_Tp, std::initializer_list<_Value>>> {

Style: the use of trailing-return-type SFINAE here is inconsistent with the use 
of template parameter SFINAE on 254.



Comment at: include/experimental/task:308
+  union {
+char __empty_;
+_Tp __value_;

These `__empty_` members seem extraneous. 



Comment at: include/experimental/task:309
+char __empty_;
+_Tp __value_;
+exception_ptr __exception_;

Should we `static_assert` that `_Tp` is a destructible object type?



Comment at: include/experimental/task:373
+template <>
+class __task_promise final : public __task_promise_base {
+  using _Handle = coroutine_handle<__task_promise>;

Do we care about `task`?



Comment at: include/experimental/task:389
+
+  void __rvalue_result() { __throw_if_exception(); }
+

Should these `__foovalue_result` members be `foo`-qualified to make them harder 
to misuse?



Comment at: include/experimental/task:407
+  using promise_type = __task_promise<_Tp>;
+
+private:

Similar to above, should we `static_assert` that `_Tp` is `void`, an lvalue 
reference type, or a destructible non-array object type?



Comment at: include/experimental/task:453
+  decltype(auto) await_resume() {
+return this->__coro_.promise().__lvalue_result();
+  }

`this->` is extraneous in these classes that derive from the concrete 
`_AwaiterBase`. (And on 470)



Comment at: include/experimental/task:458
+_LIBCPP_ASSERT(__coro_,
+   "Undefined behaviour to co_await an invalid task");
+return _Awaiter{__coro_};

Is missing a subject. How about "co_await on an invalid task has undefined 
behavior"? (And on 475)



Comment at: include/experimental/task:28
+#if defined(_LIBCPP_WARNING)
+_LIBCPP_WARNING(" cannot be

[PATCH] D59863: [HIP] Support gpu arch gfx906+sram-ecc

2019-03-27 Thread Yaxun Liu via Phabricator via cfe-commits

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



Comment at: lib/Basic/Cuda.cpp:113
+  case CudaArch::GFX906_SRAM_ECC: // TBA
+return "gfx906+sram-ecc";
   case CudaArch::GFX909: // TBA

tra wrote:
> Wording nit:
> Does it mean `+(SRAM, ECC)` or `+SRAM, -ECC` ?
> 
> From the rest of the changes I guess it's the former, but the syntax used can 
> easily be interpreted as the later.
> 
> Perhaps change the feature name to `sram_ecc` and GPU name to something less 
> verbose. `gfx906se` ?
It actually means +(SRAM-ECC).

There is actually documentation about what features are supported and their 
format

https://llvm.org/docs/AMDGPUUsage.html#code-object-target-identification
https://llvm.org/docs/AMDGPUUsage.html#amdgpu-target-features

Since the name has been used in runtime, we cannot change that.

Not all combinations are useful, therefore this patch only supports the one 
that is known to be useful.

I am considering a more generic approach, but so far I do not see its necessity 
yet.


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

https://reviews.llvm.org/D59863



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

2019-03-27 Thread Paul Robinson via Phabricator via cfe-commits

probinson added a comment.

In D59347#1444819 , @dblaikie wrote:

> In D59347#1443051 , @dblaikie wrote:
>
> > @asmith: Where's the LLVM-side change/review that goes with this, btw?
> >
> > In D59347#1442970 , @probinson 
> > wrote:
> >
> > > As a rule I would prefer flags with positive names, as it's slightly 
> > > easier to read `!isTrivial` than `!isNonTrivial`. And trivially shorter. 
> > > :-)
> >
> >
> > Fair enough - I was mostly coming at this from the "the patch that was 
> > committed should be reverted" & then we could haggle over other things, but 
> > fair point.
>
>
> Hmm, one other thought: Technically "non trivial" is perhaps more 
> accurate/less error prone. Only marking structures as "trivial" but other 
> types without that marker makes it more subtle (since not all trivial types 
> would be marked trivial - only those of a classification that means they 
> /could/ be non-trivial). Whereas marking the non-trivial types is more 
> broadly accurate.


Well, that's a reasonable point in favor of keeping the NonTrivial flag.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59347



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

[PATCH] D59874: [PR41247] Fixed parsing of private keyword in C++

2019-03-27 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia updated this revision to Diff 192485.
Anastasia added a comment.
Herald added a subscriber: jdoerfert.

Extended test case and explained the intention of the test.


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

https://reviews.llvm.org/D59874

Files:
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseTentative.cpp
  test/SemaOpenCLCXX/private-access-specifier.cpp


Index: test/SemaOpenCLCXX/private-access-specifier.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/private-access-specifier.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -pedantic -verify -fsyntax-only
+
+// Test that 'private' is not parsed as an address space qualifier
+// in regular C++ mode.
+
+struct B {
+  virtual ~B() // expected-error{{expected ';' at end of declaration list}}
+private:
+   void foo();
+   private int* i; // expected-error{{expected ':'}}
+};
+
+void bar(private int*); //expected-error{{variable has incomplete type 
'void'}} expected-error{{expected expression}}
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1410,8 +1410,13 @@
 // cv-qualifier
   case tok::kw_const:
   case tok::kw_volatile:
+return TPResult::True;
+
 // OpenCL address space qualifiers
   case tok::kw_private:
+if (!getLangOpts().OpenCL)
+  return TPResult::False;
+LLVM_FALLTHROUGH;
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4791,7 +4791,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4800,9 +4799,11 @@
   case tok::kw___read_only:
   case tok::kw___read_write:
   case tok::kw___write_only:
-
 return true;
 
+  case tok::kw_private:
+return getLangOpts().OpenCL;
+
   // C11 _Atomic
   case tok::kw__Atomic:
 return true;
@@ -4982,7 +4983,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4995,6 +4995,9 @@
 #include "clang/Basic/OpenCLImageTypes.def"
 
 return true;
+
+  case tok::kw_private:
+return getLangOpts().OpenCL;
   }
 }
 
@@ -5196,6 +5199,9 @@
 
 // OpenCL qualifiers:
 case tok::kw_private:
+  if (!getLangOpts().OpenCL)
+goto DoneWithTypeQuals;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:


Index: test/SemaOpenCLCXX/private-access-specifier.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/private-access-specifier.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -pedantic -verify -fsyntax-only
+
+// Test that 'private' is not parsed as an address space qualifier
+// in regular C++ mode.
+
+struct B {
+  virtual ~B() // expected-error{{expected ';' at end of declaration list}}
+private:
+   void foo();
+   private int* i; // expected-error{{expected ':'}}
+};
+
+void bar(private int*); //expected-error{{variable has incomplete type 'void'}} expected-error{{expected expression}}
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1410,8 +1410,13 @@
 // cv-qualifier
   case tok::kw_const:
   case tok::kw_volatile:
+return TPResult::True;
+
 // OpenCL address space qualifiers
   case tok::kw_private:
+if (!getLangOpts().OpenCL)
+  return TPResult::False;
+LLVM_FALLTHROUGH;
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4791,7 +4791,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4800,9 +4799,11 @@
   case tok::kw___read_only:
   case tok::kw___read_write:
   case tok::kw___write_only:
-
 return true;
 
+  case tok::kw_private:
+return getLangOpts().OpenCL;
+
   // C11 _Atomic
   case tok::kw__Atomic:
 return true;
@@ -4982,7 +4983,6 @@
 
   case tok::kw___kindof:
 
-  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4995,6 +4995,9 @@
 #include "clang/Basic/OpenCLImageTypes.def"
 
 return true;
+
+  case tok::kw_private:
+return getLangOpts().OpenCL;
   }
 }
 
@@ -5196,6 +5199,9 @@
 
 // OpenCL qualifiers:
 case tok::kw_private:
+  if (!getLangOpts().OpenCL)
+goto DoneWithTypeQuals;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw_

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ updated this revision to Diff 192492.
NoQ marked 2 inline comments as done.
NoQ added a comment.

Address comments. Fix printing taint in state dumps.


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

https://reviews.llvm.org/D59861

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintTag.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/lib/StaticAnalyzer/Core/TaintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/TaintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/TaintManager.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//== TaintManager.cpp -- -*- C++ -*--=//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
-
-using namespace clang;
-using namespace ento;
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index = 0;
-  return &index;
-}
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index;
-  return &index;
-}
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -16,7 +16,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -458,9 +457,6 @@
   // Print out the tracked dynamic types.
   printDynamicTypeInfo(this, Out, NL, Sep);
 
-  // Print out tainted symbols.
-  printTaint(Out, NL);
-
   // Print checker-specific data.
   Mgr.getOwningEngine().printState(Out, this, NL, Sep, LC);
 }
@@ -474,22 +470,6 @@
   print(llvm::errs());
 }
 
-void ProgramState::printTaint(raw_ostream &Out,
-  const char *NL) const {
-  TaintMapImpl TM = get();
-
-  if (!TM.isEmpty())
-Out <<"Tainted symbols:" << NL;
-
-  for (TaintMapImpl::iterator I = TM.begin(), E = TM.end(); I != E; ++I) {
-Out << I->first << " : " << I->second << NL;
-  }
-}
-
-void ProgramState::dumpTaint() const {
-  printTaint(llvm::errs());
-}
-
 AnalysisManager& ProgramState::getAnalysisManager() const {
   return stateMgr->getOwningEngine().getAnalysisManager();
 }
@@ -657,166 +637,3 @@
   }
   return true;
 }
-
-ProgramStateRef ProgramState::addTaint(const Stmt *S,
-   const LocationContext *LCtx,
-   TaintTagType Kind) const {
-  if (const Expr *E = dyn_cast_or_null(S))
-S = E->IgnoreParens();
-
-  return addTaint(getSVal(S, LCtx), Kind);
-}
-
-ProgramStateRef ProgramState::addTaint(SVal V,
-   TaintTagType Kind) const {
-  SymbolRef Sym = V.getAsSymbol();
-  if (Sym)
-return addTaint(Sym, Kind);
-
-  // If the SVal represents a structure, try to mass-taint all values within the
-  // structure. For now it only works efficiently on lazy compound values that
-  // were conjured during a conservative evaluation of a function - either as
-  // return values of functions that return structures or arrays by value, or as
-  // values of structures or arrays passed into the function by reference,
-  // directly or through pointer aliasing. Such lazy compound values are
-  // characterized by having exactly one binding in their captured store within
-  // their parent region, which is a conjured symbol default-bound to the base
-  // region of the parent region.
-  if (auto LCV = V.getAs()) {
-if (Optional binding = getStateManager().StoreMgr->getDefaultBinding(*LCV)) {
-  if (SymbolRef Sym = binding->getAsSymbol())
-return addPartialTaint(Sym, LCV->getRegion(), Kind);
-}
-  }
-
-  cons

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.h:8
+//
+//===--===//
+//

Charusso wrote:
> Outdated header-blurb.
Whooops! Thanks!!



Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.h:81
+
+void dumpTaint(ProgramStateRef State);
+

Charusso wrote:
> We left the `ProgramState::dump()' context so what about just `dump()`?
I'm doing `using namespace taint;` everywhere, so that'd make the code look 
like `dump(State)` which would look quite ambiguous to a human. Additionally, 
this function only needs to be called once, so it's not a problem if it's a bit 
long.

Also, wait, i didn't really call it even once. I.e., i forgot to implement 
`GenericTaintChecker::printState()` to print taint (now that it's not 
state-specific, there must be a checker responsible for this, and i'm casting 
my vote for `GenericTaintChecker` out of 1 potential candidates). Let me fix it.


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

https://reviews.llvm.org/D59861



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

[PATCH] D59457: [analyzer][NFC] Use capital variable names, move methods out-of-line, rename some in CheckerRegistry

2019-03-27 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 agree that we shouldn't try real hard to fix our code until we have a clear 
"party line" on how exactly do we name our variables (and ideally also how do 
we invalidate our caches), but having variables and fields consistent with each 
other makes it look much better imo, so why not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59457



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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

2019-03-27 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D59485#1444673 , @teemperor wrote:

> In D59485#1439628 , @martong wrote:
>
> > In D59485#1439570 , @martong wrote:
> >
> > > In D59485#1439390 , @teemperor 
> > > wrote:
> > >
> > > > > Well, I still don't understand how LLDB synthesis the AST. 
> > > > >  So you have a C++ module in a .pcm file. This means you could create 
> > > > > an AST from that with ASTReader from it's .clang_ast section, right? 
> > > > > In that case the AST should be complete with all type information. If 
> > > > > this was the case then I don't see why it is not possible to use 
> > > > > clang::ASTImporter to import templates and specializations, since we 
> > > > > do exactly that in CTU.
> > > > > 
> > > > > Or do you guys create the AST from the debug info, from the 
> > > > > .debug_info section of .pcm module file? And this is why AST is 
> > > > > incomplete? I've got this from 
> > > > > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> > > > >  If this is the case, then comes my naiive quiestion: Why don't you 
> > > > > use the ASTReader?
> > > >
> > > > There are no C++ modules involved in our use case beside the generic 
> > > > `std` module. No user-written code is read from a module and we don't 
> > > > have any PCM file beside the `std` module we build for the expression 
> > > > evaluator.
> > > >
> > > > We use the ASTReader to directly read the `std` module contents into 
> > > > the expression evaluation ASTContext, but this doesn't give us the 
> > > > decls for the instantiations the user has made (e.g. 
> > > > `std::vector`). We only have these user instantiations in the 
> > > > 'normal' debug info where we have a rather minimal AST (again, no 
> > > > modules are not involved in building this debug info AST). The 
> > > > `InternalImport` in the LLDB patch just stitches the module AST and the 
> > > > debug info AST together when we import something that we also have (in 
> > > > better quality from the C++ module) in the target ASTContext.
> > >
> > >
> > > Thank you for the explanation.
> > >  Now I understand you directly create specializations from the std module 
> > > and intercept the import to avoid importing broken specializations from 
> > > the debug info, this makes sense.
> >
> >
> > Really, just one last question to see the whole picture: If 
> > clang::ASTImporter were capable of handling a specialization/instantiation 
> > with missing info then we could use that. So the reason for this 
> > interception is that 
> > clang::ASTImporter::VisitClassTemplateSpecializationDecl cannot handle the 
> > specialization it receives because that or its dependent nodes has too many 
> > missing data, right?
>
>
> Feel free to ask! I'm just traveling so my internet access (and my reply 
> rate) is a bit low at the moment.
>
> Not sure if we can easily merge the logic of D59537 
>  into the ASTImporter. It has no logic at 
> all to actually determine if a source AST node has missing data but solely 
> relies on our knowledge that our AST in LLDB isn't very useful for std 
> templates. Also instantiating a template instead of importing an existing 
> instantiation seems like a very rare scenario. I'm not even sure how we would 
> test having a broken AST with Clang (the parser won't produce one, so we 
> would have to hack our own AST builder for broken nodes).
>
> If there is a reasonable way to get this logic into the ASTImporter I have no 
> objection against doing so. The only problem I see is that I can't come up 
> with a reasonable way of merging/testing the logic in the ASTImporter (and 
> what other application would benefit from it).


Raphael, thank you for your answer. This explanation makes so much more easier 
for me to understand the need for this patch.


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

https://reviews.llvm.org/D59485



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

Mostly nits and small API questions form my side.
This looks very good overall! I'm planning to take a closer look at the 
handling of macros and various AST nodes in more detail this week, but the 
approach looks solid from a higher level.




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ymandel wrote:
> ilya-biryukov wrote:
> > ymandel wrote:
> > > ilya-biryukov wrote:
> > > > ymandel wrote:
> > > > > ilya-biryukov wrote:
> > > > > > Maybe consider separating the fluent API to build the rewrite rule 
> > > > > > from the rewrite rule itself?
> > > > > > 
> > > > > > Not opposed to having the fluent builder API, this does look nice 
> > > > > > and seems to nicely align with the matcher APIs.
> > > > > > However, it is somewhat hard to figure out what can `RewriteRule` 
> > > > > > do **after** it was built when looking at the code right now.
> > > > > > ```
> > > > > > class RewriteRule {
> > > > > > public:
> > > > > >   RewriteRule(DynTypedMatcher, TextGenerator Replacement, 
> > > > > > TextGenerator Explanation);
> > > > > > 
> > > > > >   DynTypedMatcher matcher() const;
> > > > > >   Expected replacement() const;
> > > > > >   Expected explanation() const;
> > > > > > };
> > > > > > 
> > > > > > struct RewriteRuleBuilder { // Having a better name than 'Builder' 
> > > > > > would be nice.
> > > > > >   RewriteRule finish() &&; // produce the final RewriteRule.
> > > > > > 
> > > > > >   template 
> > > > > >   RewriteRuleBuilder &change(const TypedNodeId &Target,
> > > > > >   NodePart Part = NodePart::Node) &;
> > > > > >   RewriteRuleBuilder &replaceWith(TextGenerator Replacement) &;
> > > > > >   RewriteRuleBuilder &because(TextGenerator Explanation) &;
> > > > > > private:
> > > > > >   RewriteRule RuleInProgress;
> > > > > > };
> > > > > > RewriteRuleBuilder makeRewriteRule();
> > > > > > ```
> > > > > I see your point, but do you think it might be enough to improve the 
> > > > > comments on the class? My concern with a builder is the mental burden 
> > > > > on the user of another concept (the builder) and the need for an 
> > > > > extra `.build()` everywhere. To a lesser extent, I also don't love 
> > > > > the cost of an extra copy, although I doubt it matters and I suppose 
> > > > > we could support moves in the build method.
> > > > The issues with the builder interface is that it requires lots of 
> > > > boilerplate, which is hard to throw away when reading the code of the 
> > > > class. I agree that having a separate builder class is also annoying 
> > > > (more concepts, etc).
> > > > 
> > > > Keeping them separate would be my personal preference, but if you'd 
> > > > prefer to keep it in the same class than maybe move the non-builder 
> > > > pieces to the top of the class and separate the builder methods with a 
> > > > comment. 
> > > > WDYT? 
> > > > 
> > > > > To a lesser extent, I also don't love the cost of an extra copy, 
> > > > > although I doubt it matters and I suppose we could support moves in 
> > > > > the build method.
> > > > I believe we can be as efficient (up to an extra move) with builders as 
> > > > without them. If most usages are of the form `RewriteRule R = 
> > > > rewriteRule(...).change(...).replaceWith(...).because(...);`
> > > > Then we could make all builder functions return r-value reference to a 
> > > > builder and have an implicit conversion operator that would consume the 
> > > > builder, producing the final `RewriteRule`:
> > > > ```
> > > > class RewriteRuleBuilder {
> > > >   operator RewriteRule () &&;
> > > >   /// ...
> > > > };
> > > > RewriteRuleBuilder rewriteRule();
> > > > 
> > > > void addRule(RewriteRule R);
> > > > void clientCode() {
> > > >   
> > > > addRule(rewriteRule().change(Matcher).replaceWith("foo").because("bar"));
> > > > }
> > > > ```
> > > I didn't realize that implicit conversion functions are allowed. With 
> > > that, I'm fine w/ splitting.   Thanks!
> > Have you uploaded the new version? I don't seem to see the split.
> I have now.  FWIW, I've left both ref overloads, but what do you think of 
> dropping the lvalue-ref overloads and only supporting rvalue refs?  Users can 
> still pretty easily use an lvalue, just by inserting a trivial std::move() 
> around the lvalue.  
Yeah, LG, having only a single overload means less boilerplate and the 
fluent-builder APIs are mostly used exclusively as r-values anyway.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:38
+/// @{
+using ast_matchers::CXXCtorInitializerMatcher;
+using ast_matchers::DeclarationMatcher;

I'm not sure if this is in the LLVM style guide, but we might want to avoid 
introducing these names into `clang::tooling` namespaces in the headers.

My fear is that users will rely on those using without know

[clang-tools-extra] r357114 - [clang-tidy] Handle missing yaml module in run-clang-tidy.py

2019-03-27 Thread Zinovy Nis via cfe-commits

Author: zinovy.nis
Date: Wed Mar 27 12:21:32 2019
New Revision: 357114

URL: http://llvm.org/viewvc/llvm-project?rev=357114&view=rev
Log:
[clang-tidy] Handle missing yaml module in run-clang-tidy.py

The Yaml module is missing on some systems and on many of clang buildbots. 
But the test for run-clang-tidy.py doesn't fail due to 'NOT' statement masking 
a python runtime error.

This patch conditionally imports and enables the yaml module only if it's 
present in the system. 
If not, then '-export-fixes' is disabled.

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
clang-tools-extra/trunk/test/clang-tidy/bugprone-parent-virtual-call.cpp   
(props changed)
clang-tools-extra/trunk/test/clang-tidy/run-clang-tidy.cpp

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=357114&r1=357113&r2=357114&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 27 
12:21:32 2019
@@ -36,11 +36,10 @@ import tempfile
 import threading
 import traceback
 
-yaml_imported = True
 try:
   import yaml
 except ImportError:
-  yaml_imported = False
+  yaml = None
 
 is_py2 = sys.version[0] == '2'
 
@@ -144,7 +143,7 @@ def main():
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
-  if yaml_imported:
+  if yaml:
 parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
 help='Create a yaml file to store suggested fixes in, '
 'which can be applied with clang-apply-replacements.')
@@ -204,7 +203,7 @@ def main():
   max_task_count = min(len(lines_by_file), max_task_count)
 
   tmpdir = None
-  if yaml_imported and args.export_fixes:
+  if yaml and args.export_fixes:
 tmpdir = tempfile.mkdtemp()
 
   # Tasks for clang-tidy.
@@ -238,7 +237,7 @@ def main():
 # Run clang-tidy on files containing changes.
 command = [args.clang_tidy_binary]
 command.append('-line-filter=' + line_filter_json)
-if yaml_imported and args.export_fixes:
+if yaml and args.export_fixes:
   # Get a temporary file. We immediately close the handle so clang-tidy can
   # overwrite it.
   (handle, tmp_name) = tempfile.mkstemp(suffix='.yaml', dir=tmpdir)
@@ -253,7 +252,7 @@ def main():
   # Wait for all threads to be done.
   task_queue.join()
 
-  if yaml_imported and args.export_fixes:
+  if yaml and args.export_fixes:
 print('Writing fixes to ' + args.export_fixes + ' ...')
 try:
   merge_replacement_files(tmpdir, args.export_fixes)

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=357114&r1=357113&r2=357114&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Wed Mar 27 
12:21:32 2019
@@ -47,7 +47,11 @@ import sys
 import tempfile
 import threading
 import traceback
-import yaml
+
+try:
+  import yaml
+except ImportError:
+  yaml = None
 
 is_py2 = sys.version[0] == '2'
 
@@ -199,9 +203,10 @@ def main():
   'headers to output diagnostics from. Diagnostics from '
   'the main file of each translation unit are always '
   'displayed.')
-  parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes',
-  help='Create a yaml file to store suggested fixes in, '
-  'which can be applied with clang-apply-replacements.')
+  if yaml:
+parser.add_argument('-export-fixes', metavar='filename', 
dest='export_fixes',
+help='Create a yaml file to store suggested fixes in, '
+'which can be applied with clang-apply-replacements.')
   parser.add_argument('-j', type=int, default=0,
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('files', nargs='*', default=['.*'],
@@ -254,7 +259,7 @@ def main():
 max_task = multiprocessing.cpu_count()
 
   tmpdir = None
-  if args.fix or args.export_fixes:
+  if args.fix or (yaml and args.export_fixes):
 check_clang_apply_replacements_binary(args)
 tmpdir = tempfile.mkdtemp()
 
@@ -292,7 +297,7 @@ def main():
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
-  if args.export_fixes:
+  if yaml and args.export_fixes:
 print('Writing fixes to ' + args.export_fixes

[PATCH] D59734: [clang-tidy] Handle missing yaml module in run-clang-tidy.py

2019-03-27 Thread Zinovy Nis 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 rL357114: [clang-tidy] Handle missing yaml module in 
run-clang-tidy.py (authored by zinovy.nis, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59734?vs=192137&id=192495#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59734

Files:
  clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
  clang-tools-extra/trunk/test/clang-tidy/bugprone-parent-virtual-call.cpp
  clang-tools-extra/trunk/test/clang-tidy/run-clang-tidy.cpp

Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -47,7 +47,11 @@
 import tempfile
 import threading
 import traceback
-import yaml
+
+try:
+  import yaml
+except ImportError:
+  yaml = None
 
 is_py2 = sys.version[0] == '2'
 
@@ -199,9 +203,10 @@
   'headers to output diagnostics from. Diagnostics from '
   'the main file of each translation unit are always '
   'displayed.')
-  parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes',
-  help='Create a yaml file to store suggested fixes in, '
-  'which can be applied with clang-apply-replacements.')
+  if yaml:
+parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes',
+help='Create a yaml file to store suggested fixes in, '
+'which can be applied with clang-apply-replacements.')
   parser.add_argument('-j', type=int, default=0,
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('files', nargs='*', default=['.*'],
@@ -254,7 +259,7 @@
 max_task = multiprocessing.cpu_count()
 
   tmpdir = None
-  if args.fix or args.export_fixes:
+  if args.fix or (yaml and args.export_fixes):
 check_clang_apply_replacements_binary(args)
 tmpdir = tempfile.mkdtemp()
 
@@ -292,7 +297,7 @@
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
-  if args.export_fixes:
+  if yaml and args.export_fixes:
 print('Writing fixes to ' + args.export_fixes + ' ...')
 try:
   merge_replacement_files(tmpdir, args.export_fixes)
Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -36,11 +36,10 @@
 import threading
 import traceback
 
-yaml_imported = True
 try:
   import yaml
 except ImportError:
-  yaml_imported = False
+  yaml = None
 
 is_py2 = sys.version[0] == '2'
 
@@ -144,7 +143,7 @@
   default='')
   parser.add_argument('-path', dest='build_path',
   help='Path used to read a compile command database.')
-  if yaml_imported:
+  if yaml:
 parser.add_argument('-export-fixes', metavar='FILE', dest='export_fixes',
 help='Create a yaml file to store suggested fixes in, '
 'which can be applied with clang-apply-replacements.')
@@ -204,7 +203,7 @@
   max_task_count = min(len(lines_by_file), max_task_count)
 
   tmpdir = None
-  if yaml_imported and args.export_fixes:
+  if yaml and args.export_fixes:
 tmpdir = tempfile.mkdtemp()
 
   # Tasks for clang-tidy.
@@ -238,7 +237,7 @@
 # Run clang-tidy on files containing changes.
 command = [args.clang_tidy_binary]
 command.append('-line-filter=' + line_filter_json)
-if yaml_imported and args.export_fixes:
+if yaml and args.export_fixes:
   # Get a temporary file. We immediately close the handle so clang-tidy can
   # overwrite it.
   (handle, tmp_name) = tempfile.mkstemp(suffix='.yaml', dir=tmpdir)
@@ -253,7 +252,7 @@
   # Wait for all threads to be done.
   task_queue.join()
 
-  if yaml_imported and args.export_fixes:
+  if yaml and args.export_fixes:
 print('Writing fixes to ' + args.export_fixes + ' ...')
 try:
   merge_replacement_files(tmpdir, args.export_fixes)
Index: clang-tools-extra/trunk/test/clang-tidy/run-clang-tidy.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/run-clang-tidy.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/run-clang-tidy.cpp
@@ -1,3 +1,4 @@
+// RUN: %run_clang_tidy --help
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c %/t/test.cpp\",\"file\":\"%/t/test.cpp\"}]" | sed -e 's/

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ updated this revision to Diff 192501.
NoQ marked 2 inline comments as done.

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

https://reviews.llvm.org/D58367

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -91,6 +91,14 @@
  // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  vm_deallocate(port, address, size); // no-note
+  1 / 0; // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+  return KERN_SUCCESS;
+}
+
 // Make sure we find the bug when the object is destroyed within an
 // automatic destructor.
 MIG_SERVER_ROUTINE
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -201,7 +201,9 @@
   svalBuilder(StateMgr.getSValBuilder()),
   ObjCNoRet(mgr.getASTContext()),
   BR(mgr, *this),
-  VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
+  VisitedCallees(VisitedCalleesIn),
+  HowToInline(HowToInlineIn),
+  NoteTags(G.getAllocator()) {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
   if (TrimInterval != 0) {
 // Enable eager node reclamation when constructing the ExplodedGraph.
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2480,6 +2480,30 @@
   return nullptr;
 }
 
+int NoteTag::Kind = 0;
+
+void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
+  static int Tag = 0;
+  ID.AddPointer(&Tag);
+}
+
+std::shared_ptr
+TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+  BugReport &R) {
+  ProgramPoint PP = N->getLocation();
+  const NoteTag *T = dyn_cast_or_null(PP.getTag());
+  if (!T)
+return nullptr;
+
+  if (Optional Msg = T->generateMessage(BRC, R)) {
+PathDiagnosticLocation Loc =
+PathDiagnosticLocation::create(PP, BRC.getSourceManager());
+return std::make_shared(Loc, *Msg);
+  }
+
+  return nullptr;
+}
+
 void FalsePositiveRefutationBRVisitor::Profile(
 llvm::FoldingSetNodeID &ID) const {
   static int Tag = 0;
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2612,6 +2612,7 @@
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
+R->addVisitor(llvm::make_unique());
 
 BugReporterContext BRC(Reporter, ErrorGraph.BackMap);
 
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -80,43 +80,10 @@
 checkReturnAux(RS, C);
   }
 
-  class Visitor : public BugReporterVisitor {
-  public:
-void Profile(llvm::FoldingSetNodeID &ID) const {
-  static int X = 0;
-  ID.AddPointer(&X);
-}
-
-std::shared_ptr VisitNode(const ExplodedNode *N,
-BugReporterContext &BRC, BugReport &R);
-  };
 };
 } // end anonymous namespace
 
-// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits
-// specialization for this sort of types.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *)
-
-std::shared_ptr
-MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
-   BugReport &R) {
-  const auto *NewPVD = static_cast(
-  N->getState()->get());
-  const auto *OldPVD = static_cast(
-  N->getFirstPred()->getState()->get());
-  if (OldPVD == NewPVD)
-return nullptr;
-
-  assert(NewPVD && "What is deallocated cannot be un-deallocated!");
-  SmallString<64> Str;
-  llvm::raw_svector_ostream OS(Str)

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:600
+public:
+  typedef std::function
+  Callback;

Szelethus wrote:
> Prefer using.
Thx!~



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:348
+/// additional PathDiagnosticEventPieces along the path.
+class NoteTag : public ProgramPointTag {
+public:

baloghadamsoftware wrote:
> I am not sure whether `BugReporterVisitors.h` is the best place for this 
> structure. I would rather put this into `ProgramPoint.h` or maybe 
> `BugReporter.h`.
Moved to `BugReporter.h`.

`ProgramPoint.h` is too low-level, it's not even in `libStaticAnalyzer*`, so 
talking about stuff like `BugReporterContext` within it sounds a bit hard to 
get compiled.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:386-398
+  // Manage memory for NoteTag objects.
+  class Factory {
+std::vector> Tags;
+
+  public:
+const NoteTag *makeNoteTag(Callback &&Cb) {
+  // We cannot use make_unique because we cannot access the private

Szelethus wrote:
> Hmmm, did you consider the other memory management options we have in LLVM, 
> like `BumpPtrAllocator`? Sounds a bit better for this use case.
Hmm, apparently i didn't. Fxd.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400
+
+  friend class Factory;
+  friend class TagVisitor;

xazax.hun wrote:
> Isn't this redundant?
It isn't - i made a private constructor as usual.


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

https://reviews.llvm.org/D58367



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

[PATCH] D59725: Additions to creduce script

2019-03-27 Thread Amy Huang via Phabricator via cfe-commits

akhuang updated this revision to Diff 192499.
akhuang marked 3 inline comments as done.
akhuang added a comment.

change `mkstemp` to `NamedTemporaryFile` and add `decode(utf-8)` so it works on 
python3.5


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

https://reviews.llvm.org/D59725

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -1,8 +1,14 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+
+Output files:
+  *.reduced.sh -- crash reproducer with minimal arguments
+  *.reduced.cpp -- the reduced file
+  *.test.sh -- interestingness test for C-Reduce
 """
 
-from argparse import ArgumentParser
+from __future__ import print_function
+from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
 import stat
@@ -15,10 +21,14 @@
 from distutils.spawn import find_executable
 
 verbose = False
-llvm_bin = None
 creduce_cmd = None
+clang_cmd = None
 not_cmd = None
 
+def verbose_print(*args, **kwargs):
+  if verbose:
+print(*args, **kwargs)
+
 def check_file(fname):
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
@@ -33,166 +43,337 @@
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
-sys.exit("ERROR: executable %s not found" % (cmd_path))
+sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
   cmd = find_executable(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
-  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
 
-def quote_cmd(cmd):
-  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
-  for arg in cmd)
-
-def get_crash_cmd(crash_script):
-  with open(crash_script) as f:
-# Assume clang call is on the last line of the script
-line = f.readlines()[-1]
-cmd = shlex.split(line)
-
-# Overwrite the script's clang with the user's clang path
-new_clang = check_cmd('clang', llvm_bin)
-cmd[0] = pipes.quote(new_clang)
-return cmd
+  if not cmd_dir:
+cmd_dir = "$PATH"
+  sys.exit("ERROR: `%s` not found in %s" % (cmd_name, cmd_dir))
 
-def has_expected_output(crash_cmd, expected_output):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-  return all(msg in crash_output for msg in expected_output)
-
-def get_expected_output(crash_cmd):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-
-  # If there is an assertion failure, use that;
-  # otherwise use the last five stack trace functions
-  assertion_re = r'Assertion `([^\']+)\' failed'
-  assertion_match = re.search(assertion_re, crash_output)
-  if assertion_match:
-return [assertion_match.group(1)]
-  else:
-stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
-matches = re.findall(stacktrace_re, crash_output)
-return matches[-5:]
-
-def write_interestingness_test(testfile, crash_cmd, expected_output,
-   file_to_reduce):
-  filename = os.path.basename(file_to_reduce)
-  if filename not in crash_cmd:
-sys.exit("ERROR: expected %s to be in the crash command" % filename)
-
-  # Replace all instances of file_to_reduce with a command line variable
-  output = ['#!/bin/bash',
-'if [ -z "$1" ] ; then',
-'  f=%s' % (pipes.quote(filename)),
-'else',
-'  f="$1"',
-'fi']
-  cmd = ['$f' if s == filename else s for s in crash_cmd]
-
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
-  quote_cmd(cmd)))
-
-  for msg in expected_output:
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
-
-  with open(testfile, 'w') as f:
-f.write('\n'.join(output))
-  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
-
-def check_interestingness(testfile, file_to_reduce):
-  testfile = os.path.abspath(testfile)
-
-  # Check that the test considers the original file interesting
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call(testfile, stdout=devnull)
-  if returncode:
-sys.exit("The interestingness test does not pass for the original file.")
-
-  # Check that an empty file is not interesting
-  _, empty_file = tempfile.mkstemp()
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call([testfile, empty_file], stdout=devnull)
-  os.remove(empty_file)
-  if not returncode:
-sys.exit("The interestingness test passes for an empty file.")
-
-def clang_preprocess(file_to_reduce, crash_cmd, expected_output):
-  _, tmpfile = tempfile.mkstemp()
-  shutil.copy(file_to_reduce, tmpfile)

[PATCH] D59725: Additions to creduce script

2019-03-27 Thread Amy Huang via Phabricator via cfe-commits

akhuang added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:198
+# Instead of modifying the filename in the test file, just run the command
+fd, empty_file = tempfile.mkstemp()
+if self.check_expected_output(filename=empty_file):

george.burgess.iv wrote:
> Did we want to use `NamedTemporaryFile` here as rnk suggested?
> 
> (If not, you can lift the `os.close`s to immediately after this line.)
switched to using `NamedTemporaryFile` here - 



Comment at: clang/utils/creduce-clang-crash.py:206
+print("\nTrying to preprocess the source file...")
+fd, tmpfile = tempfile.mkstemp()
+

george.burgess.iv wrote:
> Similar question about `NamedTemporaryFile`.
> 
> Please note that you'll probably have to pass `delete=False`, since 
> apparently `delete=True` sets O_TEMPORARY on Windows, which... might follow 
> the file across renames? I'm unsure.
moved to `NamedTemporaryFile` with comment about `delete=False`


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

https://reviews.llvm.org/D59725



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

[PATCH] D59741: [lit] Set shlibpath_var on AIX

2019-03-27 Thread Xing Xue via Phabricator via cfe-commits

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

LGTM.  Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59741



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400
+
+  friend class Factory;
+  friend class TagVisitor;

NoQ wrote:
> xazax.hun wrote:
> > Isn't this redundant?
> It isn't - i made a private constructor as usual.
I had the impression that nested classes can access the members of their parent 
classes since C++11.


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

https://reviews.llvm.org/D58367



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

//*un-forgets to actually post comments*//

In D58367#1443830 , @Charusso wrote:

> Cool! I have found this message-semantic idea useful in Unity where every 
> `GameObject` could talk with each other in a very generic way 
> (https://docs.unity3d.com/ScriptReference/GameObject.SendMessage.html).

I mean, for instance, the whole Objective-C is built in this very manner. But 
it's not always worth it to opt in into such highly dynamic system that 
bypasses all type checks - in many cases there's a more straightforward 
solution.

In D58367#1444683 , @Szelethus wrote:

> Amazing work! Thanks!
>
> In D58367#1443783 , @NoQ wrote:
>
> > Remove memoization for now. Address a few inline comments. I'm mostly done 
> > with this first patch, did i accidentally miss anything?
> >
> > In D58367#1402796 , @Szelethus 
> > wrote:
> >
> > > 1. It would be great to have unit tests for this.
> >
> >
> > Mm, i have problems now that checker registry is super locked down: i need 
> > a bug report object => i need a checker (or at least a `CheckName`) => i 
> > need the whole `AnalysisConsumer` => i can no longer override `ASTConsumer` 
> > methods for testing purposes (because `AnalysisConsumer` is a final 
> > sub-class of `ASTConsumer`). Do you have a plan B for that?
>
>
> Saw this, will think about it! Though I'm a little unsure what you mean under 
> the checker registry being locked down.

I mean, you hunted down checker names by making sure all objects are 
constructed properly, which probably means that it's harder to construct these 
objects improperly for testing purposes. I'm not really sure - maybe it has 
always been that way :) Anyway, all i need is a `CheckName`, which is merely a 
string under the hood, but a very hard-to-obtain one.

> 
> 
>> I also generally don't see many good unit tests we could write here, that 
>> would add much to the integration tests we already have, but this 
>> memoization problem could have made a nice unit test.
> 
> I don't insist, but unlike most of the subprojects within clang, we have 
> very-very few infrastructural unit tests. I don't even find them to be that 
> important for testing purposes, but more as minimal examples: I remember that 
> `unittests/StaticAnalyzer/RegisterCustomChecker.cpp` was a godsent when I 
> worked on checker dependencies.

Yeah, this one's really nice. Another place where i find unittests to be 
important is when it comes to checking the separation of concerns, eg. the 
story behind D23963  ("`RegionStore` isn't 
responsible for the semantics of brace initialization"). I'd love to have unit 
tests for our environment/store/constraint managers and i'll definitely make 
some when i find myself implementing a new data structure of this kind.

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

https://reviews.llvm.org/D58367

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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ updated this revision to Diff 192502.
NoQ marked an inline comment as done.

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

https://reviews.llvm.org/D58367

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -91,6 +91,14 @@
  // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  vm_deallocate(port, address, size); // no-note
+  1 / 0; // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+  return KERN_SUCCESS;
+}
+
 // Make sure we find the bug when the object is destroyed within an
 // automatic destructor.
 MIG_SERVER_ROUTINE
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -201,7 +201,9 @@
   svalBuilder(StateMgr.getSValBuilder()),
   ObjCNoRet(mgr.getASTContext()),
   BR(mgr, *this),
-  VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
+  VisitedCallees(VisitedCalleesIn),
+  HowToInline(HowToInlineIn),
+  NoteTags(G.getAllocator()) {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
   if (TrimInterval != 0) {
 // Enable eager node reclamation when constructing the ExplodedGraph.
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2480,6 +2480,30 @@
   return nullptr;
 }
 
+int NoteTag::Kind = 0;
+
+void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
+  static int Tag = 0;
+  ID.AddPointer(&Tag);
+}
+
+std::shared_ptr
+TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+  BugReport &R) {
+  ProgramPoint PP = N->getLocation();
+  const NoteTag *T = dyn_cast_or_null(PP.getTag());
+  if (!T)
+return nullptr;
+
+  if (Optional Msg = T->generateMessage(BRC, R)) {
+PathDiagnosticLocation Loc =
+PathDiagnosticLocation::create(PP, BRC.getSourceManager());
+return std::make_shared(Loc, *Msg);
+  }
+
+  return nullptr;
+}
+
 void FalsePositiveRefutationBRVisitor::Profile(
 llvm::FoldingSetNodeID &ID) const {
   static int Tag = 0;
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2612,6 +2612,7 @@
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
+R->addVisitor(llvm::make_unique());
 
 BugReporterContext BRC(Reporter, ErrorGraph.BackMap);
 
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -80,43 +80,10 @@
 checkReturnAux(RS, C);
   }
 
-  class Visitor : public BugReporterVisitor {
-  public:
-void Profile(llvm::FoldingSetNodeID &ID) const {
-  static int X = 0;
-  ID.AddPointer(&X);
-}
-
-std::shared_ptr VisitNode(const ExplodedNode *N,
-BugReporterContext &BRC, BugReport &R);
-  };
 };
 } // end anonymous namespace
 
-// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits
-// specialization for this sort of types.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *)
-
-std::shared_ptr
-MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
-   BugReport &R) {
-  const auto *NewPVD = static_cast(
-  N->getState()->get());
-  const auto *OldPVD = static_cast(
-  N->getFirstPred()->getState()->get());
-  if (OldPVD == NewPVD)
-return nullptr;
-
-  assert(NewPVD && "What is deallocated cannot be un-deallocated!");
-  SmallString<64> Str;
-  llvm::raw_svector_ostream OS(Str)

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:400
+
+  friend class Factory;
+  friend class TagVisitor;

xazax.hun wrote:
> NoQ wrote:
> > xazax.hun wrote:
> > > Isn't this redundant?
> > It isn't - i made a private constructor as usual.
> I had the impression that nested classes can access the members of their 
> parent classes since C++11.
Wait, seriously?!~ Thx!


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

https://reviews.llvm.org/D58367



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

[PATCH] D59861: [analyzer] NFC: Replace Taint API with a usual inter-checker communication API?

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

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

Add a test for taint dumps.


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

https://reviews.llvm.org/D59861

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/TaintTag.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/lib/StaticAnalyzer/Core/TaintManager.cpp
  clang/test/Analysis/taint-dumps.c

Index: clang/test/Analysis/taint-dumps.c
===
--- /dev/null
+++ clang/test/Analysis/taint-dumps.c
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint\
+// RUN:-analyzer-checker=debug.ExprInspection %s\
+// RUN:  2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+int getchar();
+
+// CHECK: Tainted symbols:
+// CHECK-NEXT: conj_$2{{.*}} : 0
+int test_taint_dumps() {
+  int x = getchar();
+  clang_analyzer_printState();
+  return x;
+}
Index: clang/lib/StaticAnalyzer/Core/TaintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/TaintManager.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-//== TaintManager.cpp -- -*- C++ -*--=//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
-
-using namespace clang;
-using namespace ento;
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index = 0;
-  return &index;
-}
-
-void *ProgramStateTrait::GDMIndex() {
-  static int index;
-  return &index;
-}
Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -16,7 +16,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -458,9 +457,6 @@
   // Print out the tracked dynamic types.
   printDynamicTypeInfo(this, Out, NL, Sep);
 
-  // Print out tainted symbols.
-  printTaint(Out, NL);
-
   // Print checker-specific data.
   Mgr.getOwningEngine().printState(Out, this, NL, Sep, LC);
 }
@@ -474,22 +470,6 @@
   print(llvm::errs());
 }
 
-void ProgramState::printTaint(raw_ostream &Out,
-  const char *NL) const {
-  TaintMapImpl TM = get();
-
-  if (!TM.isEmpty())
-Out <<"Tainted symbols:" << NL;
-
-  for (TaintMapImpl::iterator I = TM.begin(), E = TM.end(); I != E; ++I) {
-Out << I->first << " : " << I->second << NL;
-  }
-}
-
-void ProgramState::dumpTaint() const {
-  printTaint(llvm::errs());
-}
-
 AnalysisManager& ProgramState::getAnalysisManager() const {
   return stateMgr->getOwningEngine().getAnalysisManager();
 }
@@ -657,166 +637,3 @@
   }
   return true;
 }
-
-ProgramStateRef ProgramState::addTaint(const Stmt *S,
-   const LocationContext *LCtx,
-   TaintTagType Kind) const {
-  if (const Expr *E = dyn_cast_or_null(S))
-S = E->IgnoreParens();
-
-  return addTaint(getSVal(S, LCtx), Kind);
-}
-
-ProgramStateRef ProgramState::addTaint(SVal V,
-   TaintTagType Kind) const {
-  SymbolRef Sym = V.getAsSymbol();
-  if (Sym)
-return addTaint(Sym, Kind);
-
-  // If the SVal represents a structure, try to mass-taint all values within the
-  // structure. For now it only works efficiently on lazy compound values that
-  // were conjured during a conservative evaluation of a function - either as
-  // return values of functions that return structures or arrays by value, or as
-  // values of struc

[PATCH] D59863: [HIP] Support gpu arch gfx906+sram-ecc

2019-03-27 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: wdng.

This should not be a new device name. This is also not how the features should 
be passed to the backend. These should be added to the function IR directly


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

https://reviews.llvm.org/D59863



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

[PATCH] D59899: gn build: Add some build files for clangd

2019-03-27 Thread Nico Weber via Phabricator via cfe-commits

thakis created this revision.
thakis added a reviewer: mbonadei.
Herald added subscribers: jdoerfert, jfb, kadircet, arphaman, jkorous, MaskRay, 
ioeric, ilya-biryukov, mgorny, srhines.
Herald added a project: LLVM.

Enough to build the clangd binaries, but this is still missing build
files for:

- fuzzer
- indexer
- index/dex/dexp
- benchmarks
- xpc


https://reviews.llvm.org/D59899

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  llvm/utils/gn/build/libs/atomic/BUILD.gn
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-apply-replacements/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
@@ -0,0 +1,50 @@
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+
+declare_args() {
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = false
+}
+
+write_cmake_config("features") {
+  # FIXME: Try moving Features.inc.in to tools, seems like a better location.
+  input = "../Features.inc.in"
+  output = "$target_gen_dir/Features.inc"
+  values = []
+  if (clangd_build_xpc) {
+values += [ "CLANGD_BUILD_XPC=1" ]
+  } else {
+values += [ "CLANGD_BUILD_XPC=0" ]
+  }
+}
+
+executable("clangd") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+":features",
+"//clang-tools-extra/clang-tidy",
+"//clang-tools-extra/clangd",
+"//clang-tools-extra/clangd/refactor/tweaks",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Sema",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+
+  include_dirs = [
+"..",
+
+# To pick up the generated inc files.
+"$target_gen_dir",
+  ]
+  sources = [
+"ClangdMain.cpp",
+  ]
+
+  if (clangd_build_xpc) {
+# FIXME: Depend on clangdXpcJsonConversions, clangdXpcTransport
+  }
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn
@@ -0,0 +1,16 @@
+# A target containing all code tweaks (i.e. mini-refactorings) provided by
+# clangd.
+# Built as a source_set to make sure the linker does not remove global
+# constructors that register individual tweaks in a global registry.
+source_set("tweaks") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//clang/lib/AST",
+"//clang/lib/Tooling/Core",
+  ]
+  include_dirs = [ "../.." ]
+  sources = [
+"SwapIfBranches.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
@@ -0,0 +1,95 @@
+static_library("clangd") {
+  output_name = "clangDaemon"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-tidy/abseil",
+"//clang-tools-extra/clang-tidy/android",
+"//clang-tools-extra/clang-tidy/boost",
+"//clang-tools-extra/clang-tidy/bugprone",
+"//clang-tools-extra/clang-tidy/cert",
+"//clang-tools-extra/clang-tidy/cppcoreguidelines",
+"//clang-tools-extra/clang-tidy/fuchsia",
+"//clang-tools-extra/clang-tidy/google",
+"//clang-tools-extra/clang-tidy/hicpp",
+"//clang-tools-extra/clang-tidy/llvm",
+"//clang-tools-extra/clang-tidy/misc",
+"//clang-tools-extra/clang-tidy/modernize",
+"//clang-tools-extra/clang-tidy/objc",
+"//clang-tools-extra/clang-tidy/performance",
+"//clang-tools-extra/clang-tidy/portability",
+"//clang-tools-extra/clang-tidy/readability",
+"//clang-tools-extra/clang-tidy/zircon",
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Driver",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Index",
+"//clang/lib/Lex",
+"//clang/lib/Sema",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling/Core",
+"//clang/lib/Tooling/Inclusions",
+"//clang/lib/Tooling/Refactoring",
+"//llvm/lib/Support",
+"//llvm/utils/gn/build/libs/atomic",
+"//llvm/utils/gn/build/libs/pthread",
+  ]
+  include_dirs = [ "." ]
+  sources = [
+"AST.cpp",
+"Cancellation.cpp",
+"ClangdLSPServer.cpp",
+"ClangdServer.cpp",
+"ClangdUnit.cpp",
+"CodeComplete.cpp",
+"CodeCompletionStrings.cpp",
+"Compiler.cpp",
+"Context.cpp",
+"Diagnostics.cpp",
+"DraftStore.cpp",
+"ExpectedTypes.cpp",
+"FS.c

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

2019-03-27 Thread Michael Liao via Phabricator via cfe-commits

hliao created this revision.
hliao added reviewers: rjmccall, tra, yaxunl.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

- Non-null checking is triggered during prototype substitution from a template 
instantiation, if expressions in `decltype` contains nullptr chcking. Skip 
non-null checking in that case as the protype is not finalized yet. Also, the 
nullptr checking in `decltype` is only used for type inspection instead of 
codegen. Ignoring that is harmless.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59900

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaTemplate/decltype.cpp


Index: clang/test/SemaTemplate/decltype.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/decltype.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag
+
+// expected-no-diagnostics
+template 
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11592,6 +11592,9 @@
   }
 
   if (const auto *FD = dyn_cast(PV->getDeclContext())) {
+// Skip function template not specialized yet.
+if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+  return;
 auto ParamIter = llvm::find(FD->parameters(), PV);
 assert(ParamIter != FD->param_end());
 unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);


Index: clang/test/SemaTemplate/decltype.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/decltype.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag
+
+// expected-no-diagnostics
+template 
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11592,6 +11592,9 @@
   }
 
   if (const auto *FD = dyn_cast(PV->getDeclContext())) {
+// Skip function template not specialized yet.
+if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+  return;
 auto ParamIter = llvm::find(FD->parameters(), PV);
 assert(ParamIter != FD->param_end());
 unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59901: [analyzer] PR41239: Fix a crash on invalid source location in NoStoreFuncVisitor.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso, alexfh.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet, 
kristof.beyls, javed.absar.
Herald added a project: clang.

It turns out that `SourceManager::isInSystemHeader()` crashes when an invalid 
source location is passed into it. Invalid source locations are relatively 
common: not only they come from body farms, but also, say, any function in C 
that didn't come with a forward declaration would have an implicit forward 
declaration with invalid source locations.

Not sure if this deserves to be fixed in `SourceManager`, but there's anyway a 
more comfy API for us to use in the Static Analyzer: 
`CallEvent::isInSystemHeader()`, so i just used that.


Repository:
  rC Clang

https://reviews.llvm.org/D59901

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c


Index: clang/test/Analysis/diagnostics/no-store-func-path-notes.c
===
--- clang/test/Analysis/diagnostics/no-store-func-path-notes.c
+++ clang/test/Analysis/diagnostics/no-store-func-path-notes.c
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core -analyzer-output=text 
-verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core 
-analyzer-output=text\
+// RUN: -verify %s
 
 typedef __typeof(sizeof(int)) size_t;
 void *memset(void *__s, int __c, size_t __n);
@@ -244,3 +245,12 @@
   return z; // expected-warning{{Undefined or garbage value returned to 
caller}}
 // expected-note@-1{{Undefined or garbage value returned to 
caller}}
 }
+
+void test_implicit_function_decl(int *x) {
+  if (x) {} // expected-note{{Assuming 'x' is null}}
+// expected-note@-1{{Taking false branch}}
+  implicit_function(x);
+  *x = 4; // expected-warning{{Dereference of null pointer (loaded from 
variable 'x')}}
+  // expected-note@-1{{Dereference of null pointer (loaded from 
variable 'x')}}
+}
+int implicit_function(int *y) {}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -322,7 +322,7 @@
 CallEventRef<> Call =
 BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-if (SM.isInSystemHeader(Call->getDecl()->getSourceRange().getBegin()))
+if (Call->isInSystemHeader())
   return nullptr;
 
 // Region of interest corresponds to an IVar, exiting a method


Index: clang/test/Analysis/diagnostics/no-store-func-path-notes.c
===
--- clang/test/Analysis/diagnostics/no-store-func-path-notes.c
+++ clang/test/Analysis/diagnostics/no-store-func-path-notes.c
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text\
+// RUN: -verify %s
 
 typedef __typeof(sizeof(int)) size_t;
 void *memset(void *__s, int __c, size_t __n);
@@ -244,3 +245,12 @@
   return z; // expected-warning{{Undefined or garbage value returned to caller}}
 // expected-note@-1{{Undefined or garbage value returned to caller}}
 }
+
+void test_implicit_function_decl(int *x) {
+  if (x) {} // expected-note{{Assuming 'x' is null}}
+// expected-note@-1{{Taking false branch}}
+  implicit_function(x);
+  *x = 4; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+  // expected-note@-1{{Dereference of null pointer (loaded from variable 'x')}}
+}
+int implicit_function(int *y) {}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -322,7 +322,7 @@
 CallEventRef<> Call =
 BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-if (SM.isInSystemHeader(Call->getDecl()->getSourceRange().getBegin()))
+if (Call->isInSystemHeader())
   return nullptr;
 
 // Region of interest corresponds to an IVar, exiting a method
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59901: [analyzer] PR41239: Fix a crash on invalid source location in NoStoreFuncVisitor.

2019-03-27 Thread Csaba Dabis via Phabricator via cfe-commits

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

Nice solution.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59901



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

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

2019-03-27 Thread Ana Pazos via Phabricator via cfe-commits

apazos added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:1813
+}
+  } else if (Arg *A = Args.getLastArg(options::OPT_G)) {
+SmallDataLimit = A->getValue();

Why do you we need to set a default? It will cause the optimization to be on 
now, and I thought we want to first prioritize the globals we want to consider 
for the optimization.
I think you should check for msmall-data-limit flag occurring with G,  fpic and 
mcmodel flags and print warning in all cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

[libclc] r357125 - travis: Add LLVM-8 build

2019-03-27 Thread Jan Vesely via cfe-commits

Author: jvesely
Date: Wed Mar 27 14:28:31 2019
New Revision: 357125

URL: http://llvm.org/viewvc/llvm-project?rev=357125&view=rev
Log:
travis: Add LLVM-8 build

Reviewer: Tom Stellard
Signed-off-by: Jan Vesely 

Modified:
libclc/trunk/.travis.yml

Modified: libclc/trunk/.travis.yml
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/.travis.yml?rev=357125&r1=357124&r2=357125&view=diff
==
--- libclc/trunk/.travis.yml (original)
+++ libclc/trunk/.travis.yml Wed Mar 27 14:28:31 2019
@@ -87,6 +87,23 @@ matrix:
 - llvm-7-dev
 - clang-7
 - env:
+- LABEL="make gcc LLVM-8"
+- LLVM_VERSION=8
+- CHECK_FILES="barts-r600--.bc cayman-r600--.bc cedar-r600--.bc 
cypress-r600--.bc tahiti-amdgcn--.bc amdgcn--amdhsa.bc 
tahiti-amdgcn-mesa-mesa3d.bc nvptx--nvidiacl.bc nvptx64--nvidiacl.bc"
+- MATRIX_EVAL="CC=gcc-6 && CXX=g++-6"
+  addons:
+apt:
+  sources:
+- sourceline: 'deb http://apt.llvm.org/trusty/ 
llvm-toolchain-trusty-8 main'
+  key_url: https://apt.llvm.org/llvm-snapshot.gpg.key
+- ubuntu-toolchain-r-test
+  packages:
+- libedit-dev
+- g++-6
+# From sources above
+- llvm-8-dev
+- clang-8
+- env:
 - LABEL="cmake gcc LLVM-3.9"
 - LLVM_VERSION=3.9
 - CHECK_FILES="barts-r600--.bc cayman-r600--.bc cedar-r600--.bc 
cypress-r600--.bc tahiti-amdgcn--.bc amdgcn--amdhsa.bc nvptx--nvidiacl.bc 
nvptx64--nvidiacl.bc"
@@ -163,6 +180,23 @@ matrix:
 # From sources above
 - llvm-7-dev
 - clang-7
+- env:
+- LABEL="cmake gcc LLVM-8"
+- LLVM_VERSION=8
+- CHECK_FILES="barts-r600--.bc cayman-r600--.bc cedar-r600--.bc 
cypress-r600--.bc tahiti-amdgcn--.bc amdgcn--amdhsa.bc 
tahiti-amdgcn-mesa-mesa3d.bc nvptx--nvidiacl.bc nvptx64--nvidiacl.bc"
+- MATRIX_EVAL="CC=gcc-6 && CXX=g++-6"
+  addons:
+apt:
+  sources:
+- sourceline: 'deb http://apt.llvm.org/trusty/ 
llvm-toolchain-trusty-8 main'
+  key_url: https://apt.llvm.org/llvm-snapshot.gpg.key
+- ubuntu-toolchain-r-test
+  packages:
+- libedit-dev
+- g++-6
+# From sources above
+- llvm-8-dev
+- clang-8
 
 before_install:
 - eval "${MATRIX_EVAL}"


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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-27 Thread Anton Afanasyev via Phabricator via cfe-commits

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

takuto.ikuta wrote:
> anton-afanasyev wrote:
> > takuto.ikuta wrote:
> > > Remove StringRef?
> > I use `StringRef("")` to explicitly cast to one of overloaded 
> > `TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, 
> > fuction_ref(std::string()))`.
> I think function_ref(std::string()) does not have constructor receiving 
> StringRef, so I feel explicit cast is redundant.
> 
The compiler tries to use function_ref<..>(Callable&&, ...) constructor and 
instantiates `function_ref::function_ref`, so 
one gets error like:
```
error: call to constructor of 'llvm::TimeTraceScope' is ambiguous
llvm::TimeTraceScope TimeScope("Frontend", "");
 ^ ~~
.../include/llvm/Support/TimeProfiler.h:54:3: note: candidate constructor
  TimeTraceScope(StringRef Name, StringRef Detail) {
  ^
.../include/llvm/Support/TimeProfiler.h:58:3: note: candidate constructor
  TimeTraceScope(StringRef Name, llvm::function_ref Detail) {
  ^

```


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-27 Thread Anton Afanasyev via Phabricator via cfe-commits

anton-afanasyev updated this revision to Diff 192525.
anton-afanasyev marked 10 inline comments as done.

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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,185 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char &C: Src) {
+switch (C) {
+case '"':
+case '/':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  OS += '\\';
+  OS += C;
+  break;
+default:
+  if (isPrint(C)) {
+OS += C;
+  }
+}
+  }
+  return OS;
+}
+
+typedef duration DurationType;
+typedef std::pair NameAndDuration;
+
+struct Entry {
+  time_point Start;
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};
+
+struct TimeTraceProfiler {
+  TimeTraceProfiler() {
+Stack.reserve(8);
+Entries.reserve(128);
+StartTime = steady_clock::now();
+  }
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));
+  }
+
+  void end() {
+assert(!Stack.empty() && "Must call begin() first");
+auto &E = Stack.back();
+E.Duration = steady_clock::now() - E.Start;
+
+// Only include sections longer than 500us.
+if (duration_cast(E.Duration).count() > 500)
+  Entries.emplace_back(E);
+
+// Track total time taken by each "name", but only the topmost levels of
+// them; e.g. if there's a template instantiation that instantiates other
+// templates from within, we only want to add the topmost one. "topmost"
+// happens to be the ones that don't have any currently open entries above
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry &Val) {
+  return Val.Name == E.Name;
+}) == Stack.rend()) {
+  TotalPerName[E.Name] += E.Duration;
+  CountPerName[E.Name]++;
+}
+
+Stack.pop_back();
+  }
+
+  void Write(std::unique_ptr &OS) {
+assert(Stack.empty() &&
+   "All profiler sections should be ended when calling Write");
+
+*OS << "{ \"traceEvents\": [\n";
+
+// Emit all events for the main flame graph.
+for (const auto &E : Entries) {
+  auto StartUs = duration_cast(E.Start - StartTime).count();
+  auto DurUs = duration_cast(E.Duration).count();
+  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+  << ", \"dur\":" << DurUs << ", \"name\":\""
+  << escapeString(E.Name) << "\", \"args\":{ \"detail\":\""
+  << escapeString(E.Detail) << "\"} },\n";
+}
+
+// Emit totals by section name as additional "thread" events, sorted from
+// longest one.
+int Tid = 1;
+std::vector SortedTotals;
+SortedTotals.reserve(TotalPerName.size());
+for (const auto &E : TotalPerName) {
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration &A, const NameAndDuration &B) {
+return A.second > B.second;
+  });
+for (const auto &E : SortedTotals) {
+  auto DurUs = duration_cast(E.second).count();
+  *OS << "{ \"pid\":1, \"tid\":" << Tid << ", \"ph\"

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

2019-03-27 Thread Artem Belevich via Phabricator via cfe-commits

tra added subscribers: jlebar, rsmith.
tra added a comment.

@rsmith, @jlebar I'm out of my depth here and could use some language lawyering 
help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59900



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

r357127 - Revert "[WebAssembly] Don't use default GetLinkerPath"

2019-03-27 Thread Derek Schuff via cfe-commits

Author: dschuff
Date: Wed Mar 27 15:22:18 2019
New Revision: 357127

URL: http://llvm.org/viewvc/llvm-project?rev=357127&view=rev
Log:
Revert "[WebAssembly] Don't use default GetLinkerPath"

This reverts commit 4dcf3acce6d7455fd079d8e57441906ca2bad254.
(reverts LLVM SVN r356953)

Modified:
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Driver/wasm-toolchain.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=357127&r1=357126&r2=357127&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Wed Mar 27 15:22:18 2019
@@ -12,8 +12,6 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Option/ArgList.h"
 
 using namespace clang::driver;
@@ -38,25 +36,6 @@ bool wasm::Linker::isLinkJob() const { r
 
 bool wasm::Linker::hasIntegratedCPP() const { return false; }
 
-std::string wasm::Linker::getLinkerPath(const ArgList &Args) const {
-  const ToolChain &ToolChain = getToolChain();
-  if (const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
-StringRef UseLinker = A->getValue();
-if (!UseLinker.empty()) {
-  if (llvm::sys::path::is_absolute(UseLinker) &&
-  llvm::sys::fs::can_execute(UseLinker))
-return UseLinker;
-
-  // Accept 'lld', and 'ld' as aliases for the default linker
-  if (UseLinker != "lld" && UseLinker != "ld")
-ToolChain.getDriver().Diag(diag::err_drv_invalid_linker_name)
-<< A->getAsString(Args);
-}
-  }
-
-  return ToolChain.getDefaultLinker();
-}
-
 void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 const InputInfo &Output,
 const InputInfoList &Inputs,
@@ -64,7 +43,7 @@ void wasm::Linker::ConstructJob(Compilat
 const char *LinkingOutput) const {
 
   const ToolChain &ToolChain = getToolChain();
-  const char *Linker = Args.MakeArgString(getLinkerPath(Args));
+  const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
   ArgStringList CmdArgs;
 
   if (Args.hasArg(options::OPT_s))

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.h?rev=357127&r1=357126&r2=357127&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.h Wed Mar 27 15:22:18 2019
@@ -23,7 +23,6 @@ public:
   explicit Linker(const ToolChain &TC);
   bool isLinkJob() const override;
   bool hasIntegratedCPP() const override;
-  std::string getLinkerPath(const llvm::opt::ArgList &Args) const;
   void ConstructJob(Compilation &C, const JobAction &JA,
 const InputInfo &Output, const InputInfoList &Inputs,
 const llvm::opt::ArgList &TCArgs,

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=357127&r1=357126&r2=357127&view=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Wed Mar 27 15:22:18 2019
@@ -12,25 +12,25 @@
 
 // A basic C link command-line with unknown OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with unknown OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

2019-03-27 Thread Derek Schuff via Phabricator via cfe-commits

dschuff added a comment.

Reverted in rG039be787914610c28cba45c4557454e0a96939ab 
. Caused a 
strange error with the waterfall sysroot's build of libcxx: 
https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/8917800786005174656/+/steps/libcxx/0/stdout


Repository:
  rC Clang

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

https://reviews.llvm.org/D59743



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

[PATCH] D55007: [Analyzer] Constraint Manager - Calculate Effective Range for Differences

2019-03-27 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.

What i was trying to say with my last comment is that i guess i'd rather go for 
option (1) because with that `getRange()` remains the single source of truth, 
which is comfy.

I agree this shouldn't really be blocking the patch - sorry for stalling! - i'm 
hopefully slowly getting better at not stalling.

Generally i would have went for saving some memory and expensive ImmutableMap 
lookups by canonicalizing the key as much as possible.

Do we want to add the opposite test

  void effective_range_2(int m, int n) {
assert(m - n <= 0);
assert(n - m <= 0);
clang_analyzer_eval(m - n == 0); // expected-warning{{TRUE}} 
expected-warning{{FALSE}}
clang_analyzer_eval(n - m == 0); // expected-warning{{TRUE}} 
expected-warning{{FALSE}}
  }

...where the `FALSE` case corresponds to `m - n == INT_MIN`?




Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:483
+
+  // If we have range set sotred for both A - B and B - A then calculate the
+  // effective range set by intersecting the range set for A - B and the

`sotred` -> `stored` :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55007



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

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

2019-03-27 Thread Justin Lebar via Phabricator via cfe-commits

jlebar added a comment.

I uh...  I also think this is an @rsmith question, I have no idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59900



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

[PATCH] D53343: [Driver] Default Android toolchains to noexecstack.

2019-03-27 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits

pirama accepted this revision.
pirama added inline comments.
This revision is now accepted and ready to land.
Herald added a project: clang.



Comment at: include/clang/Driver/ToolChain.h:393
 
+  /// Test whether this toolchaind defaults to non-executable stacks.
+  virtual bool isNoExecStackDefault() const;

typo: toolchain


Repository:
  rC Clang

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

https://reviews.llvm.org/D53343



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

2019-03-27 Thread Ryan Piantedosi via Phabricator via cfe-commits

Dosi-Dough updated this revision to Diff 192541.
Dosi-Dough marked 4 inline comments as done.
Dosi-Dough added a comment.

fixed bracketed return and added updated license


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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() {}
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h
=

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

2019-03-27 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/abseil/WrapUniqueCheck.h:26
+private:
+ // std::string getArgs(const SourceManager *SM, const CallExpr *MemExpr);
+

Please remove obsolete commend and private keyword.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:5
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 

Please separate with empty line.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:16
+  private:
+A() {}
+  };

Please use = default;


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

https://reviews.llvm.org/D57435



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

[PATCH] D59914: [analyzer] MIGChecker: Add support for more deallocator APIs.

2019-03-27 Thread Artem Dergachev via Phabricator via cfe-commits

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

Most of these were obtained by invoking

  $ find . -name '*.defs' -exec grep destructor {} \; | sed s/'.*destructor: 
'// | sed s/'(.*)'// | awk '{print "CALL(1, 0, " $1 "),"}' | sort -u

in XNU .

rdar://problem/49358317


Repository:
  rC Clang

https://reviews.llvm.org/D59914

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -15,11 +15,15 @@
 typedef unsigned vm_size_t;
 typedef void *ipc_space_t;
 typedef unsigned long io_user_reference_t;
+typedef struct ipc_port *ipc_port_t;
+typedef unsigned mach_port_t;
+typedef uint32_t UInt32;
 
 kern_return_t vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 kern_return_t mach_vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 void mig_deallocate(vm_address_t, vm_size_t);
 kern_return_t mach_port_deallocate(ipc_space_t, mach_port_name_t);
+void ipc_port_release(ipc_port_t);
 
 #define MIG_SERVER_ROUTINE __attribute__((mig_server_routine))
 
@@ -44,12 +48,17 @@
 class IOUserClient {
 public:
   static IOReturn releaseAsyncReference64(OSAsyncReference64);
+  static IOReturn releaseNotificationPort(mach_port_t port);
 
   MIG_SERVER_ROUTINE
-  virtual IOReturn externalMethod(uint32_t selector, IOExternalMethodArguments *arguments,
-  IOExternalMethodDispatch *dispatch = 0, OSObject *target = 0, void *reference = 0);
-};
+  virtual IOReturn externalMethod(
+  uint32_t selector, IOExternalMethodArguments *arguments,
+  IOExternalMethodDispatch *dispatch = 0, OSObject *target = 0,
+  void *reference = 0);
 
+  MIG_SERVER_ROUTINE
+  virtual IOReturn registerNotificationPort(mach_port_t, UInt32, UInt32);
+};
 
 // Tests.
 
@@ -190,6 +199,13 @@
  // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t test_ipc_port_release(ipc_port_t port) {
+  ipc_port_release(port); // expected-note{{Value passed through parameter 'port' is deallocated}}
+  return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+			   // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
+}
+
 // Let's try the C++11 attribute spelling syntax as well.
 [[clang::mig_server_routine]]
 IOReturn test_releaseAsyncReference64(IOExternalMethodArguments *arguments) {
@@ -214,4 +230,10 @@
 return kIOReturnError;  // expected-warning{{MIG callback fails with error after deallocating argument value}}
 // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
   }
+
+  IOReturn registerNotificationPort(mach_port_t port, UInt32 x, UInt32 y) {
+releaseNotificationPort(port); // expected-note{{Value passed through parameter 'port' is deallocated}}
+return kIOReturnError; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+   // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
+  }
 };
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -54,11 +54,33 @@
   CALL(3, 1, "mach_vm_deallocate"),
   CALL(2, 0, "mig_deallocate"),
   CALL(2, 1, "mach_port_deallocate"),
+  CALL(1, 0, "device_deallocate"),
+  CALL(1, 0, "iokit_remove_connect_reference"),
+  CALL(1, 0, "iokit_remove_reference"),
+  CALL(1, 0, "iokit_release_port"),
+  CALL(1, 0, "ipc_port_release"),
+  CALL(1, 0, "ipc_port_release_sonce"),
+  CALL(1, 0, "ipc_voucher_attr_control_release"),
+  CALL(1, 0, "ipc_voucher_release"),
+  CALL(1, 0, "lock_set_dereference"),
+  CALL(1, 0, "memory_object_control_deallocate"),
+  CALL(1, 0, "pset_deallocate"),
+  CALL(1, 0, "semaphore_dereference"),
+  CALL(1, 0, "space_deallocate"),
+  CALL(1, 0, "space_inspect_deallocate"),
+  CALL(1, 0, "task_deallocate"),
+  CALL(1, 0, "task_inspect_deallocate"),
+  CALL(1, 0, "task_name_deallocate"),
+  CALL(1, 0, "thread_deallocate"),
+  CALL(1, 0, "thread_inspect_deallocate"),
+  CALL(1, 0, "upl_deallocate"),
+  CALL(1, 0, "vm_map_deallocate"),
   // E.g., if the checker sees a method 'releaseAsyncReference64()' that is
   // defined on class 'IOUserClient' that ta

1 2 >

1 - 100 of 101 matches

Mail list logo