[PATCH] D62839: [clangd] Index API and implementations for relations

2019-06-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/index/Index.h:107
+  virtual void
+  relations(const SymbolID , index::SymbolRole Predicate,
+llvm::function_ref Callback) const = 0;

kadircet wrote:
> We might need additional options, like limiting number of results. Could you 
> instead accept a `RelsRequest` object? You can check `RefsRequest` for a 
> sample
I called it `RelationsRequest` to avoid visual confusion with `RefsRequest`.



Comment at: clang-tools-extra/clangd/index/MemIndex.h:29
   this->Refs.try_emplace(R.first, R.second.begin(), R.second.end());
+RelationSlab::Builder Builder;
+for (const Relation  : Relations)

kadircet wrote:
> why not just store `densemap<,arrayref>` ?
I used `std::vector` because it wasn't clear where the array would live 
otherwise.



Comment at: clang-tools-extra/clangd/index/Merge.cpp:126
+llvm::function_ref Callback) const {
+  // Return results from both indexes but avoid duplicates.
+  llvm::DenseSet SeenObjects;

kadircet wrote:
> avoiding deduplicates is not enough, we also wouldn't want to report stale 
> relations coming from static index.
> 
> Could you check the logic in `MergedIndex::refs`, and hopefully refactor it 
> into a class to share between these two?
The description in `MergedIndex::refs()` says:

```
  // We don't want duplicated refs from the static/dynamic indexes,
  // and we can't reliably deduplicate them because offsets may differ slightly.
  // We consider the dynamic index authoritative and report all its refs,
  // and only report static index refs from other files.
```

It seems to me that the problem of "can't reliably deduplicate them because 
offsets may differ slightly" doesn't apply to relations, as there are no 
offsets involved. So, is there still a need to have additional logic here?

If so, what would the logic be -- don't return an object O1 from the static 
index if we've returned an object O2 from the dynamic index defined in the same 
file as O1? (Note that to implement this, we'd have to additionally look up 
each SymbolID we return in the symbol table, as we don't otherwise know what 
file the object is located in.)



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:28
+RelationSlab Rels) {
   auto Size = Symbols.bytes() + Refs.bytes();
+  // There is no need to include "Rels" in Data because the relations are self-

kadircet wrote:
> `+rels.size()`
`Size` is only the backing data size, so if we don't include the relations in 
the backing data, we shouldn't include `Rels.size()`, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62839



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


[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-06 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D62850



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


[PATCH] D62839: [clangd] Index API and implementations for relations

2019-06-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 203495.
nridge marked 16 inline comments as done.
nridge added a comment.

Addressed most review comments, also added some tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62839

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/IndexAction.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/Relation.h
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/indexer/IndexerMain.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/IndexActionTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp

Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -119,8 +119,9 @@
   auto Token = std::make_shared();
   std::weak_ptr WeakToken = Token;
 
-  SwapIndex S(llvm::make_unique(
-  SymbolSlab(), RefSlab(), std::move(Token), /*BackingDataSize=*/0));
+  SwapIndex S(llvm::make_unique(SymbolSlab(), RefSlab(),
+  RelationSlab(), std::move(Token),
+  /*BackingDataSize=*/0));
   EXPECT_FALSE(WeakToken.expired());  // Current MemIndex keeps it alive.
   S.reset(llvm::make_unique()); // Now the MemIndex is destroyed.
   EXPECT_TRUE(WeakToken.expired());   // So the token is too.
@@ -132,12 +133,13 @@
   FuzzyFindRequest Req;
   Req.Query = "2";
   Req.AnyScope = true;
-  MemIndex I(Symbols, RefSlab());
+  MemIndex I(Symbols, RefSlab(), RelationSlab());
   EXPECT_THAT(match(I, Req), ElementsAre("2"));
 }
 
 TEST(MemIndexTest, MemIndexLimitedNumMatches) {
-  auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
+  auto I =
+  MemIndex::build(generateNumSymbols(0, 100), RefSlab(), RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "5";
   Req.AnyScope = true;
@@ -152,7 +154,7 @@
 TEST(MemIndexTest, FuzzyMatch) {
   auto I = MemIndex::build(
   generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}),
-  RefSlab());
+  RefSlab(), RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "lol";
   Req.AnyScope = true;
@@ -162,8 +164,8 @@
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithoutSpecificScope) {
-  auto I =
-  MemIndex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab());
+  auto I = MemIndex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+   RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.AnyScope = true;
@@ -171,8 +173,8 @@
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithGlobalScope) {
-  auto I =
-  MemIndex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab());
+  auto I = MemIndex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+   RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {""};
@@ -181,7 +183,8 @@
 
 TEST(MemIndexTest, MatchQualifiedNamesWithOneScope) {
   auto I = MemIndex::build(
-  generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}), RefSlab());
+  generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}), RefSlab(),
+  RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::"};
@@ -190,7 +193,8 @@
 
 TEST(MemIndexTest, MatchQualifiedNamesWithMultipleScopes) {
   auto I = MemIndex::build(
-  generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), RefSlab());
+  generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), RefSlab(),
+  RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::", "b::"};
@@ -198,7 +202,8 @@
 }
 
 TEST(MemIndexTest, NoMatchNestedScopes) {
-  auto I = MemIndex::build(generateSymbols({"a::y1", "a::b::y2"}), RefSlab());
+  auto I = MemIndex::build(generateSymbols({"a::y1", "a::b::y2"}), RefSlab(),
+   RelationSlab());
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a::"};
@@ -206,7 +211,8 @@
 }
 
 TEST(MemIndexTest, IgnoreCases) {
-  auto I = 

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-06-06 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments.



Comment at: clang-tools-extra/trunk/clangd/CodeComplete.cpp:925
+// FIXME: this should only be set on CK_CurrentParameter.
+Signal.ContainsActiveParameter = true;
+  }

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > uabelho wrote:
> > > Hi!
> > > 
> > > gcc (7.4) warns on this code:
> > > 
> > > ```
> > > error: parameter 'Signal' set but not used 
> > > [-Werror=unused-but-set-parameter]
> > >   SignatureQualitySignals Signal) const {
> > >   ^~
> > > ```
> > > Should Signal be a reference? Or is it specifically not a reference right 
> > > now due to the FIXME?
> > Should be a reference, thanks for bringing that up! I'll send a fix.
> Removed this flag altogether in r362686, it is not used and has we have no 
> tests for it.
Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62476



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


[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

2019-06-06 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.

Ok! I'll be happy to have this addressed incrementally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62949



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


[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

2019-06-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D62949#1533574 , @NoQ wrote:

> I think we should:
>
> - Pre-normalize our expected outputs so that we didn't have to normalize them 
> in every run-line.


Okay; I think this might be possible to do in a separate patch that does not 
depend on this set.

> - Treat the lack of newline in plist output as a bug and try to fix it.

A quick look at this seems to point at `clang/lib/ARCMigrate/PlistReporter.cpp` 
and `clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62949



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


[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

2019-06-06 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.

Aha, ok, right!

I wouldn't be surprised if some of these are entirely unused.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62946



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


[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Wei Xiao via Phabricator via cfe-commits
wxiao3 added a comment.

Thanks for the suggestions!
I have updated the patch accordingly.

Ok for merge now?


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

https://reviews.llvm.org/D59744



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


[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

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

In D62946#1533592 , @NoQ wrote:

> Fair enough!
>
> Tests are welcome.


Is it possible? E.g. I have not found any case for `PostCondition` and may it 
requires 10+ of dot dumps which is very space-consuming. I think test to 
printing something is only required on their respective consumers side, which 
is D62761 , so later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62946



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


[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Wei Xiao via Phabricator via cfe-commits
wxiao3 updated this revision to Diff 203492.
wxiao3 edited the summary of this revision.
wxiao3 added a reviewer: mgorny.

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

https://reviews.llvm.org/D59744

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/x86_32-arguments-linux.c
  test/CodeGen/x86_32-m64.c

Index: test/CodeGen/x86_32-m64.c
===
--- /dev/null
+++ test/CodeGen/x86_32-m64.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-netbsd -target-cpu pentium4 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,NETBSD
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WIN32
+
+#include 
+__m64 m64;
+void callee(__m64 __m1, __m64 __m2);
+__m64 caller(__m64 __m1, __m64 __m2)
+{
+// LINUX-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// LINUX: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// LINUX: ret x86_mmx
+// NETBSD-LABEL: define x86_mmx @caller(x86_mmx %__m1.coerce, x86_mmx %__m2.coerce)
+// NETBSD: tail call void @callee(x86_mmx %__m2.coerce, x86_mmx %__m1.coerce)
+// NETBSD: ret x86_mmx
+// DARWIN-LABEL: define i64 @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// DARWIN: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// DARWIN: ret i64
+// IAMCU-LABEL: define <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// IAMCU: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// IAMCU: ret <1 x i64>
+// WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 %__m2.coerce)
+// WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// WIN32: ret <1 x i64>
+  callee(__m2, __m1);
+  return m64;
+}
Index: test/CodeGen/x86_32-arguments-linux.c
===
--- test/CodeGen/x86_32-arguments-linux.c
+++ test/CodeGen/x86_32-arguments-linux.c
@@ -3,7 +3,7 @@
 
 // CHECK-LABEL: define void @f56(
 // CHECK: i8 signext %a0, %struct.s56_0* byval(%struct.s56_0) align 4 %a1,
-// CHECK: i64 %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
+// CHECK: x86_mmx %a2.coerce, %struct.s56_1* byval(%struct.s56_1) align 4,
 // CHECK: <1 x double> %a4, %struct.s56_2* byval(%struct.s56_2) align 4,
 // CHECK: <4 x i32> %a6, %struct.s56_3* byval(%struct.s56_3) align 4,
 // CHECK: <2 x double> %a8, %struct.s56_4* byval(%struct.s56_4) align 4,
@@ -12,7 +12,7 @@
 
 // CHECK: call void (i32, ...) @f56_0(i32 1,
 // CHECK: i32 %{{.*}}, %struct.s56_0* byval(%struct.s56_0) align 4 %{{[^ ]*}},
-// CHECK: i64 %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
+// CHECK: x86_mmx %{{[^ ]*}}, %struct.s56_1* byval(%struct.s56_1) align 4 %{{[^ ]*}},
 // CHECK: <1 x double> %{{[^ ]*}}, %struct.s56_2* byval(%struct.s56_2) align 4 %{{[^ ]*}},
 // CHECK: <4 x i32> %{{[^ ]*}}, %struct.s56_3* byval(%struct.s56_3) align 4 %{{[^ ]*}},
 // CHECK: <2 x double> %{{[^ ]*}}, %struct.s56_4* byval(%struct.s56_4) align 4 %{{[^ ]*}},
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -915,14 +915,6 @@
: ABIArgInfo::getDirect());
 }
 
-/// IsX86_MMXType - Return true if this is an MMX type.
-bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
-  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
-cast(IRType)->getElementType()->isIntegerTy() &&
-IRType->getScalarSizeInBits() != 64;
-}
-
 static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction ,
   StringRef Constraint,
   llvm::Type* Ty) {
@@ -1010,7 +1002,10 @@
   bool IsWin32StructABI;
   bool IsSoftFloatABI;
   bool IsMCUABI;
+  bool IsLinuxABI;
+  bool IsNetBSDABI;
   unsigned DefaultNumRegisterParameters;
+  bool IsMMXEnabled;
 
   static bool isRegisterSize(unsigned Size) {
 return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
@@ -1070,13 +1065,17 @@
 
   X86_32ABIInfo(CodeGen::CodeGenTypes , bool DarwinVectorABI,
 bool RetSmallStructInRegABI, bool Win32StructABI,
-unsigned NumRegisterParameters, bool SoftFloatABI)
+unsigned NumRegisterParameters, bool SoftFloatABI,
+bool MMXEnabled)
 : SwiftABIInfo(CGT), 

[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

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

Fair enough!

Tests are welcome.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62946



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


[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

In D59555#1514602 , @NoQ wrote:

> I'm still in doubts on how to connect your work with the `CallDescription` 
> effort. I'll think more about that.


I guess i'll just make a yaml reader for `CallDescription`s as soon as the 
interface settles down a bit, and then propose you to switch to using it.




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:804-805
+  auto *Checker = mgr.registerChecker();
+  StringRef ConfigFile =
+  mgr.getAnalyzerOptions().getCheckerStringOption(Checker, "Config", "");
+  llvm::Optional Config =

I think i'll softly advocate for a more centralized format that doesn't require 
every checker to implement an option for just that purpose.

Will you be happy with a global analyzer flag, eg. `-analyzer-config 
api-yaml=/home/foo/analyzer.yaml` and then:
```lang=yaml
Checker:
Name: alpha.security.taint.TaintPropagation
Config:
Propagations:
...
```
with possibly multiple checkers in the same file? I guess we can change it 
later if you don't mind breaking flag compatibility.



Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:16-17
+  if (std::error_code ec = Buffer.getError()) {
+llvm::errs() << "Error when getting TaintPropagation's config file '"
+ << ConfigFile << "': " << ec.message() << '\n';
+return {};

I believe we should emit a compile error-like diagnostic here. One of the good 
things about compile errors would be that GUIs like scan-build would notify 
their users about compile errors in a friendly manner, while dumps to 
`llvm::errs()` will be completely ignored.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59555



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


[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

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

Thanks!

I think we should:

- Pre-normalize our expected outputs so that we didn't have to normalize them 
in every run-line.
- Treat the lack of newline in plist output as a bug and try to fix it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62949



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


[PATCH] D62885: [analyzer] Add werror flag for analyzer warnings

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

Looks great! Feel free to add a Driver flag as well (i.e., --analyzer-werror or 
something like that) so that not to type `-Xclang` every time.

In D62885#1530573 , @xazax.hun wrote:

> The only problem I see with this approach is that it is an all or nothing 
> thing at the moment. Most of the checks in CSA can have false positives and 
> people usually do not want to fail a build due to a false positive finding. 
> This would force the users to do two separate passes with the static 
> analyzer, one with the checks as errors enabled and one with the rest of the 
> checks. The former run should not include any path sensitive checks as they 
> are never free of false positives (due to the infeasible path problem).


It is a well-respected workflow to keep the codebase "analyzer-clean" 
(completely free of static analyzer warnings) at all costs. If you find true 
positives important enough, you'll be willing to suppress false positives as 
long as it allows you to address true positives as soon as they're introduced. 
It is also much cheaper to suppress false positives as you trigger them than to 
occasionally dig and triage. And it's a straightforward way to never look at 
the same false positive twice.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62885



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


[PATCH] D62926: [analyzer] ReturnVisitor: Bypass constructing objects to see inlined calls

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

Aha, nice, that's much cleaner! Indeed, we have to skip the 
construct-expression, which is going to be on our way to the program point of 
new-expression which corresponds to calling the allocator, despite being nested 
within the new-expression. An annoying corner-case.

I'd like to have a test in which the constructor is //inlined//. I suspect that 
we'll have to skip the whole inlined call in this case, and it'll be a lot of 
various program points. We'll most likely have to consult location contexts to 
skip everything within the constructor's stack frame.

In the test cases that you've updated the constructor isn't inlined because 
it's an array-new operator and we don't bother inlining 10 default constructors 
(@Szelethus: a nice application of data flow analysis might be to find out if 
the default constructor always initializes the structure in the same way, say 
all nulls, so we could evaluate one constructor and default-bind it to the 
whole array in this case; @baloghadamsoftware: this in turn reminds me that i 
still need to do something about D22374 ).


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

https://reviews.llvm.org/D62926



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


[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

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

Ah, that positive!

No, i don't think this is a valid way to suppress it. I'll tease you a bit more 
though, and only give a hint: the null value that we're null-dereferencing was 
in fact assumed to be null //in the top frame//, not within any inline 
function. Therefore the whole "Returning pointer..." note is completely 
misleading: it is irrelevant what value we return from `getSuperRegion()`, we 
might have as well evaluated it conservatively, the report would still have 
been emitted.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62978



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


[PATCH] D62835: [X86] -march=cooperlake (clang)

2019-06-06 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D62835



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


[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

2019-06-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

I think will be good idea to replace //upgrade// with //modernize// to be 
consistent with similar checks in other module.




Comment at: 
clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp:170
+
+llvm::StringRef getNewMethodName(llvm::StringRef CurrentName) {
+  std::pair ReplacementMap[] = {

Function should be static instead on placed in anonymous namespace. See LLVM 
Coding Guidelines. Same for other places.



Comment at: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h:13
+#include "../ClangTidyCheck.h"
+
+#include 

Please remove unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst:13
+
+The affected APIs are :
+

Please remove space before colon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62977



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


Buildmaster cleaning/ removing abandoned builders

2019-06-06 Thread Galina Kistanova via cfe-commits
Hello everyone,

I am going to remove builders/slaves which are off-line for a long time now
and seem abandoned.
Here is the list:

http://lab.llvm.org:8011/builders/clang-openbsd-amd64
http://lab.llvm.org:8011/builders/clang-native-aarch64-full
http://lab.llvm.org:8011/builders/clang-bpf-build
http://lab.llvm.org:8011/builders/llvm-mips-linux
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-fast
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-parallel-fast
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-detect-only
http://lab.llvm.org:8011/builders/lldb-amd64-ninja-freebsd11

If there are any objections please speak up ASAP!

Thanks

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


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

2019-06-06 Thread Charles Zhang via Phabricator via cfe-commits
czhang added a comment.

In D62829#1533345 , @lebedev.ri wrote:

> Hmm, but there already is clang's `-Wglobal-constructors`, that fires on some 
> of these:
>  https://godbolt.org/z/rSnNuu
>  You are sure those extra warning this check produces ontop of 
> `-Wglobal-constructors` are correct?
>  If so, maybe `-Wglobal-constructors` should be extended instead?


Yes, this change is for variables with static lifetimes that are not 
necessarily globally scoped. The name -global-constructors would be a misnomer. 
In addition, this check is intended purely for header files, for reasons 
documented in the .rst file. Although similar, I do not believe this check 
would make sense as an extension to -global-constructors.


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

https://reviews.llvm.org/D62829



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


[PATCH] D60974: Clang IFSO driver action.

2019-06-06 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 203465.
plotfi added a comment.

git mv'ed hidden-class-inheritance.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// 

[PATCH] D60974: Clang IFSO driver action.

2019-06-06 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 203464.
plotfi added a comment.

Updated hidden parent/child class inheritance cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/public-hidden.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | FileCheck %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck -check-prefix=CHECK-TAPI %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | \
+// RUN: llvm-readelf -s - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+#define HIDDEN  __attribute__((__visibility__(("hidden"
+#define DEFAULT __attribute__((__visibility__(("default"
+
+// CHECK-TAPI-NOT: _ZNK1Q5func1Ev
+// CHECK-TAPI-NOT: _ZNK1Q5func2Ev
+// CHECK-SYMBOLS-DAG: NOTYPE  GLOBAL HIDDEN   {{.*}} _ZNK1Q5func1Ev
+// 

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

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

Hmm, but there already is clang's `-Wglobal-constructors`, that fires on some 
of these:
https://godbolt.org/z/rSnNuu
You are sure those extra warning this check produces ontop of 
`-Wglobal-constructors` are correct?
If so, maybe `-Wglobal-constructors` should be extended instead?


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

https://reviews.llvm.org/D62829



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


r362757 - Factor out duplicated code building a MemberExpr and marking it

2019-06-06 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Jun  6 16:24:18 2019
New Revision: 362757

URL: http://llvm.org/viewvc/llvm-project?rev=362757=rev
Log:
Factor out duplicated code building a MemberExpr and marking it
referenced.

This reinstates r362563, reverted in r362597.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=362757=362756=362757=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Jun  6 16:24:18 2019
@@ -4510,6 +4510,23 @@ public:
UnqualifiedId ,
Decl *ObjCImpDecl);
 
+  MemberExpr *
+  BuildMemberExpr(Expr *Base, bool IsArrow, SourceLocation OpLoc,
+  const CXXScopeSpec *SS, SourceLocation TemplateKWLoc,
+  ValueDecl *Member, DeclAccessPair FoundDecl,
+  bool HadMultipleCandidates,
+  const DeclarationNameInfo , QualType Ty,
+  ExprValueKind VK, ExprObjectKind OK,
+  const TemplateArgumentListInfo *TemplateArgs = nullptr);
+  MemberExpr *
+  BuildMemberExpr(Expr *Base, bool IsArrow, SourceLocation OpLoc,
+  NestedNameSpecifierLoc NNS, SourceLocation TemplateKWLoc,
+  ValueDecl *Member, DeclAccessPair FoundDecl,
+  bool HadMultipleCandidates,
+  const DeclarationNameInfo , QualType Ty,
+  ExprValueKind VK, ExprObjectKind OK,
+  const TemplateArgumentListInfo *TemplateArgs = nullptr);
+
   void ActOnDefaultCtorInitializers(Decl *CDtorDecl);
   bool ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
FunctionDecl *FDecl,

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=362757=362756=362757=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Jun  6 16:24:18 2019
@@ -7189,15 +7189,12 @@ ExprResult Sema::BuildCXXMemberCallExpr(
 }
   }
 
-  MemberExpr *ME = MemberExpr::Create(
-  Context, Exp.get(), /*IsArrow=*/false, SourceLocation(),
-  NestedNameSpecifierLoc(), SourceLocation(), Method,
-  DeclAccessPair::make(FoundDecl, FoundDecl->getAccess()),
-  DeclarationNameInfo(), /*TemplateArgs=*/nullptr, Context.BoundMemberTy,
-  VK_RValue, OK_Ordinary);
-  if (HadMultipleCandidates)
-ME->setHadMultipleCandidates(true);
-  MarkMemberReferenced(ME);
+  MemberExpr *ME =
+  BuildMemberExpr(Exp.get(), /*IsArrow=*/false, SourceLocation(),
+  NestedNameSpecifierLoc(), SourceLocation(), Method,
+  DeclAccessPair::make(FoundDecl, FoundDecl->getAccess()),
+  HadMultipleCandidates, DeclarationNameInfo(),
+  Context.BoundMemberTy, VK_RValue, OK_Ordinary);
 
   QualType ResultType = Method->getReturnType();
   ExprValueKind VK = Expr::getValueKindForType(ResultType);

Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=362757=362756=362757=diff
==
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Thu Jun  6 16:24:18 2019
@@ -893,18 +893,31 @@ BuildMSPropertyRefExpr(Sema , Expr *Ba
NameInfo.getLoc());
 }
 
-/// Build a MemberExpr AST node.
-static MemberExpr *BuildMemberExpr(
-Sema , ASTContext , Expr *Base, bool isArrow,
-SourceLocation OpLoc, const CXXScopeSpec , SourceLocation TemplateKWLoc,
-ValueDecl *Member, DeclAccessPair FoundDecl,
-const DeclarationNameInfo , QualType Ty, ExprValueKind VK,
-ExprObjectKind OK, const TemplateArgumentListInfo *TemplateArgs = nullptr) 
{
-  assert((!isArrow || Base->isRValue()) && "-> base must be a pointer rvalue");
-  MemberExpr *E = MemberExpr::Create(
-  C, Base, isArrow, OpLoc, SS.getWithLocInContext(C), TemplateKWLoc, 
Member,
-  FoundDecl, MemberNameInfo, TemplateArgs, Ty, VK, OK);
-  SemaRef.MarkMemberReferenced(E);
+MemberExpr *Sema::BuildMemberExpr(
+Expr *Base, bool IsArrow, SourceLocation OpLoc, const CXXScopeSpec *SS,
+SourceLocation TemplateKWLoc, ValueDecl *Member, DeclAccessPair FoundDecl,
+bool HadMultipleCandidates, const DeclarationNameInfo ,
+QualType Ty, ExprValueKind VK, ExprObjectKind OK,
+const TemplateArgumentListInfo *TemplateArgs) {
+  NestedNameSpecifierLoc NNS =
+  SS ? SS->getWithLocInContext(Context) : 

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

2019-06-06 Thread Charles Zhang via Phabricator via cfe-commits
czhang marked an inline comment as done.
czhang added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40
+return;
+  Finder->addMatcher(varDecl().bind("var"), this);
+}

lebedev.ri wrote:
> Most of the matching should be done here, not in `check()`.
I believe that there is no way to use the AST matching language to express 
hasConstantDeclaration.


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

https://reviews.llvm.org/D62829



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


r362756 - Convert MemberExpr creation and serialization to work the same way as

2019-06-06 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Jun  6 16:24:15 2019
New Revision: 362756

URL: http://llvm.org/viewvc/llvm-project?rev=362756=rev
Log:
Convert MemberExpr creation and serialization to work the same way as
most / all other Expr subclasses.

This reinstates r362551, reverted in r362597, with a fix to a bug that
caused MemberExprs to sometimes have a null FoundDecl after a round-trip
through an AST file.

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/PCH/cxx-templates.cpp
cfe/trunk/test/PCH/cxx-templates.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=362756=362755=362756=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Jun  6 16:24:15 2019
@@ -2735,6 +2735,7 @@ class MemberExpr final
 ASTTemplateKWAndArgsInfo,
 TemplateArgumentLoc> {
   friend class ASTReader;
+  friend class ASTStmtReader;
   friend class ASTStmtWriter;
   friend TrailingObjects;
 
@@ -2769,49 +2770,38 @@ class MemberExpr final
 return MemberExprBits.HasTemplateKWAndArgsInfo;
   }
 
-public:
-  MemberExpr(Expr *base, bool isarrow, SourceLocation operatorloc,
- ValueDecl *memberdecl, const DeclarationNameInfo ,
- QualType ty, ExprValueKind VK, ExprObjectKind OK)
-  : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
- base->isValueDependent(), base->isInstantiationDependent(),
- base->containsUnexpandedParameterPack()),
-Base(base), MemberDecl(memberdecl), MemberDNLoc(NameInfo.getInfo()),
-MemberLoc(NameInfo.getLoc()) {
-assert(memberdecl->getDeclName() == NameInfo.getName());
-MemberExprBits.IsArrow = isarrow;
-MemberExprBits.HasQualifierOrFoundDecl = false;
-MemberExprBits.HasTemplateKWAndArgsInfo = false;
-MemberExprBits.HadMultipleCandidates = false;
-MemberExprBits.OperatorLoc = operatorloc;
-  }
-
-  // NOTE: this constructor should be used only when it is known that
-  // the member name can not provide additional syntactic info
-  // (i.e., source locations for C++ operator names or type source info
-  // for constructors, destructors and conversion operators).
-  MemberExpr(Expr *base, bool isarrow, SourceLocation operatorloc,
- ValueDecl *memberdecl, SourceLocation l, QualType ty,
- ExprValueKind VK, ExprObjectKind OK)
-  : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
- base->isValueDependent(), base->isInstantiationDependent(),
- base->containsUnexpandedParameterPack()),
-Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l) {
-MemberExprBits.IsArrow = isarrow;
-MemberExprBits.HasQualifierOrFoundDecl = false;
-MemberExprBits.HasTemplateKWAndArgsInfo = false;
-MemberExprBits.HadMultipleCandidates = false;
-MemberExprBits.OperatorLoc = operatorloc;
-  }
+  MemberExpr(Expr *Base, bool IsArrow, SourceLocation OperatorLoc,
+ ValueDecl *MemberDecl, const DeclarationNameInfo ,
+ QualType T, ExprValueKind VK, ExprObjectKind OK);
+  MemberExpr(EmptyShell Empty)
+  : Expr(MemberExprClass, Empty), Base(), MemberDecl() {}
 
-  static MemberExpr *Create(const ASTContext , Expr *base, bool isarrow,
+public:
+  static MemberExpr *Create(const ASTContext , Expr *Base, bool IsArrow,
 SourceLocation OperatorLoc,
 NestedNameSpecifierLoc QualifierLoc,
-SourceLocation TemplateKWLoc, ValueDecl 
*memberdecl,
-DeclAccessPair founddecl,
+SourceLocation TemplateKWLoc, ValueDecl 
*MemberDecl,
+DeclAccessPair FoundDecl,
 DeclarationNameInfo MemberNameInfo,
-const TemplateArgumentListInfo *targs, QualType ty,
-ExprValueKind VK, ExprObjectKind OK);
+const TemplateArgumentListInfo *TemplateArgs,
+QualType T, ExprValueKind VK, ExprObjectKind OK);
+
+  /// Create an implicit MemberExpr, with no location, qualifier, template
+  /// arguments, and so on.
+  static MemberExpr *CreateImplicit(const ASTContext , Expr *Base,
+bool IsArrow, ValueDecl *MemberDecl,
+QualType T, ExprValueKind VK,
+  

[PATCH] D50147: clang-format: support external styles

2019-06-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

One thing that's unclear to me is whether your aim is to

1. solve a concrete problem for your organization
2. solve a family of problems for similar organizations
3. add a new way of configuring styles for many types of users/projects

If it's 1) I think this is very reasonable and we should focus on finding the 
simplest mechanism that will work. Is it possible to always set an environment 
variable, or a build-time parameter, or install the style file in a well-known 
absolute path (is there really a mix of win/unix users?)
If it's 2), I think what's missing is evidence that other orgs have similar 
problems and requirements.
Option 3) is interesting, but much more ambitious and needs to start with a 
design doc that discusses the use cases, alternatives, and implications. 
Certainly anything involving fetching styles via git/http falls into this 
bucket, I think.

@klimek in case I'm way off base here, or there have been past discussions that 
shed light on this.

With that in mind, I'd be very happy to approve the build time config and/or an 
env variable, as long as they're off by default. It's easy to turn them on 
later, but not easy to turn them off.
If they're going to be on by default, I think we need a strong reason.

In D50147#1513166 , @Typz wrote:

> In D50147#1310146 , @sammccall wrote:
>
> > - respecting platform conventions
>
>
> I knew this one was coming: the patch is clearly not complete on this aspect, 
> but I pushed it already to get an early feedback on the generic goal/approach.
>  This definitely needs some fixing, and to be honest I was hoping there was 
> already something in LLVM code base on this topic (e.g. list of standard 
> path, access to base installation path...) : I tried to look. but did not 
> find anything yet. Any advice?


There's lots of code in Driver that manipulates search paths in 
platform-specific ways :-) Most of my experience with that urges me to avoid it 
if at all possible, and otherwise keep it very simple.

In D50147#1513166 , @Typz wrote:

> > - understanding how distro packaging is going to work
>
> I don't understand what you mean exactly. With build-time configuration, the 
> package can be customized  to look in the appropriate places for the specific 
> distribution.
>
> Can you please clarify the issues you see?


There's a mechanism, but how is it to be used? Will/should projects with a 
style guide provide style packages for distros? Or should these be part of the 
"official" clang-format package? 
If separate packages exist, how much is it going to confuse users that 
clang-format will silently format the same project with a `.clang-format` file 
different ways depending on what's installed?

> (by the way, ideally I would like to eventually further extend this patch to 
> support retrieval of external styles through url : e.g. to get style from a 
> central server, through http, git)
> 
>> One possibility to reduce the scope here: search for unknown names on under 
>> `$CLANG_FORMAT_STYLES` if the variable is set. That way it can be set by 
>> administrators within an org if appropriate, but clang-format doesn't have 
>> to have opinions about the policy here, and all binaries still behave the 
>> same.
> 
> I don't think having no search path by default (if the env var does not exist 
> or is empty) is so nice, but I can definitely add such an environment 
> variable override.




Comment at: lib/Format/Format.cpp:1080
+ llvm::vfs::FileSystem *FS = nullptr) {
+  if (!FS) {
+FS = llvm::vfs::getRealFileSystem().get();

This fallback silently changes callers of getPredefinedStyle() with no FS to 
read from the real filesystem (if the name doesn't match anything).
This seems bad for embedders, and it's not obvious what their workaround is. 
I'd suggest we either want to change the signature (e.g. by not having a 
default param) and force them to choose, or make nullptr mean no FS and 
document the inconsistency with getStyle().


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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


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

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

Addressed comments.


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

https://reviews.llvm.org/D62829

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

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

[PATCH] D62697: AMDGPU: Disable errno by default

2019-06-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

ping


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

https://reviews.llvm.org/D62697



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


[PATCH] D62696: AMDGPU: Use AMDGPU toolchain for other OSes

2019-06-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

ping


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

https://reviews.llvm.org/D62696



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


[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

2019-06-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

clang currently disallows fields of non-trivial types (e.g., `__strong`) in 
unions in C mode since it's not possible for the compiler to determine how the 
unions should be initialized, destructed, or copied.

This patch adds support for a new attribute `non_trivial_union_member`, which 
causes fields annotated with the attribute to be trivial when computing the 
trivialness of the containing union. This means the users are responsible for 
patching up the code so that the union is initialized, destructed, and copied 
in a functionally correct way.

rdar://problem/50591731


Repository:
  rC Clang

https://reviews.llvm.org/D62988

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/Decl.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenObjC/strong-in-c-struct.m
  test/SemaObjC/attr-non-trivial-union-member.m

Index: test/SemaObjC/attr-non-trivial-union-member.m
===
--- /dev/null
+++ test/SemaObjC/attr-non-trivial-union-member.m
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -verify -Wno-objc-root-class %s
+
+int x __attribute__((non_trivial_union_member)); // expected-warning {{'non_trivial_union_member' attribute only applies to field in union}}
+
+struct S0 {
+  int __attribute__((non_trivial_union_member)) f0; // expected-warning {{'non_trivial_union_member' attribute only applies to field in union}}
+};
+
+struct S1 {
+  id f0;
+};
+
+union U0 {
+  id f0; // expected-error {{ARC forbids Objective-C objects in union}}
+};
+
+union U1 {
+  struct S1 f0; // expected-error {{non-trivial C types are disallowed in union}}
+};
+
+union U2 {
+  id __attribute__((non_trivial_union_member)) f0;
+};
+
+union U3 {
+  union U2 f0;
+  struct S1 __attribute__((non_trivial_union_member)) f1;
+};
Index: test/CodeGenObjC/strong-in-c-struct.m
===
--- test/CodeGenObjC/strong-in-c-struct.m
+++ test/CodeGenObjC/strong-in-c-struct.m
@@ -80,6 +80,10 @@
   volatile int a[16];
 } VolatileArray ;
 
+typedef union {
+  id __attribute__((non_trivial_union_member)) f0;
+} NonTrivialAttrUnion;
+
 #endif
 
 #ifdef USESTRUCT
@@ -712,4 +716,19 @@
   VolatileArray t = *a;
 }
 
+// CHECK-LABEL: define void @test_non_trivial_union_member(i64 %
+// CHECK: %[[A:.*]] = alloca %[[UNION_NONTRIVIALATTRUNION:.*]], align 8
+// CHECK: %[[T:.*]] = alloca %[[UNION_NONTRIVIALATTRUNION]], align 8
+// CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[UNION_NONTRIVIALATTRUNION]], %[[UNION_NONTRIVIALATTRUNION]]* %[[A]], i32 0, i32 0
+// CHECK: %[[COERCE_VAL_IP:.*]] = inttoptr i64 %[[A_COERCE]] to i8*
+// CHECK: store i8* %[[COERCE_VAL_IP]], i8** %[[COERCE_DIVE]], align 8
+// CHECK: %[[V0:.*]] = bitcast %[[UNION_NONTRIVIALATTRUNION]]* %[[T]] to i8*
+// CHECK: %[[V1:.*]] = bitcast %[[UNION_NONTRIVIALATTRUNION]]* %[[A]] to i8*
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[V0]], i8* align 8 %[[V1]], i64 8, i1 false)
+// CHECK: ret void
+
+void test_non_trivial_union_member(NonTrivialAttrUnion a) {
+  NonTrivialAttrUnion t = a;
+}
+
 #endif /* USESTRUCT */
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7044,6 +7044,9 @@
   case ParsedAttr::AT_TransparentUnion:
 handleTransparentUnionAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_NonTrivialUnionMember:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_ObjCException:
 handleSimpleAttribute(S, D, AL);
 break;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16038,7 +16038,7 @@
   if (Record && FDTTy->getDecl()->hasVolatileMember())
 Record->setHasVolatileMember(true);
   if (Record && Record->isUnion() &&
-  FD->getType().isNonTrivialPrimitiveCType(Context))
+  FD->hasNonTrivialPrimitiveCType(Context))
 Diag(FD->getLocation(),
  diag::err_nontrivial_primitive_type_in_union);
 } else if (FDTy->isObjCObjectType()) {
@@ -16049,7 +16049,8 @@
   FD->setType(T);
 } else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
Record && !ObjCFieldLifetimeErrReported && Record->isUnion() &&
-   !getLangOpts().CPlusPlus) {
+   !getLangOpts().CPlusPlus &&
+   !FD->hasAttr()) {
   // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
@@ -16088,14 +16089,17 @@
 
 if (Record && !getLangOpts().CPlusPlus && 

[PATCH] D62970: [clang-doc] De-duplicate comments

2019-06-06 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett added a comment.

In D62970#1533229 , @jakehehrlich 
wrote:

> Actually if we can make Description a hash set that would be best. I think 
> using std::set would require an awkward < operator to be defined. There 
> should be standard ways of getting hashes out of all the string and vector 
> types and you can use hash_combine to combine all of these into a single hash.


This SGTM -- this would actually make it a bit more deterministic in the 
ordering, since we'd no longer depend on the order of file load, even.




Comment at: clang-tools-extra/clang-doc/Representation.cpp:139-140
 DefLoc = std::move(Other.DefLoc);
   // Unconditionally extend the list of locations, since we want all of them.
   std::move(Other.Loc.begin(), Other.Loc.end(), std::back_inserter(Loc));
   mergeBase(std::move(Other));

While you're here, could you actually do the same for Location? That is, make 
it a hash set on `SymbolInfo` and dedup here?


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

https://reviews.llvm.org/D62970



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


[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

2019-06-06 Thread Bob Haarman via Phabricator via cfe-commits
inglorion accepted this revision.
inglorion added a comment.
This revision is now accepted and ready to land.

Thank you. Given that proceeding with the wrong value will result in either an 
undefined reference or a reference to what might well be the wrong function at 
link time, I think making this an error (as you've done) is strictly better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62975



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


[PATCH] D61790: [C++20] add Basic consteval specifier

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

Thank you!




Comment at: clang/include/clang/AST/DeclBase.h:1497
+
+/// kind of Contexpr specifier as defined by ConstexprSpecKind.
+uint64_t ConstexprKind : 2;

"kind" -> "Kind"
"Contexpr" -> "constexpr"



Comment at: clang/lib/AST/DeclPrinter.cpp:615
+  Out << "constexpr ";
+if (D->isConsteval() && !D->isExplicitlyDefaulted())
+  Out << "consteval ";

The only way a defaulted function can be `consteval` is if `consteval` was 
literally written on the declaration, so I think we should print out the 
`consteval` keyword for all `consteval` functions regardless of whether they're 
defaulted.



Comment at: clang/lib/Parse/ParseDecl.cpp:2491
+  if (DS.hasConstexprSpecifier() && DSC != DeclSpecContext::DSC_condition) {
 Diag(DS.getConstexprSpecLoc(), diag::err_typename_invalid_constexpr);
 DS.ClearConstexprSpec();

Should this say which specifier was used? Or do we somehow reject eg. 
`sizeof(consteval int)` before we get here?



Comment at: clang/lib/Parse/ParseExprCXX.cpp:1152-1154
 P.Diag(ConstexprLoc, !P.getLangOpts().CPlusPlus17
  ? diag::ext_constexpr_on_lambda_cxx17
  : diag::warn_cxx14_compat_constexpr_on_lambda);

We should produce a `-Wc++17-compat` diagnostic similar to this for uses of 
`consteval`.



Comment at: clang/lib/Sema/DeclSpec.cpp:1296-1297
   << (TypeSpecType == TST_char16 ? "char16_t" : "char32_t");
-  if (Constexpr_specified)
+  if (hasConstexprSpecifier())
 S.Diag(ConstexprLoc, diag::warn_cxx98_compat_constexpr);
 

For `consteval` we should produce an "incompatible with C++ standards before 
C++2a" diagnostic, not an "incompatible with C++98" diagnostic.



Comment at: clang/lib/Sema/SemaDecl.cpp:4300
+// the declaration of a function or function template
+bool isConsteval = DS.getConstexprSpecifier() == CSK_consteval;
 if (Tag)

Please capitalize this variable name.



Comment at: clang/lib/Sema/SemaDecl.cpp:6664
+Diag(D.getDeclSpec().getConstexprSpecLoc(),
+ diag::err_constexpr_no_declarators)
+<< /*consteval*/ 1;

Please consider renaming this diagnostic; `err_constexpr_no_declarators` 
doesn't describe the problem here. Maybe `err_constexpr_wrong_decl_kind` or 
something?



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6655
   MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
 Diag(MD->getBeginLoc(), diag::err_incorrect_defaulted_constexpr) << CSM;
 // FIXME: Explain why the special member can't be constexpr.

rsmith wrote:
> The diagnostic text here is:
> 
> ```
> error: defaulted definition of  is not constexpr
> ```
> 
> which might be confusing if the user wrote `consteval thing = default;`. 
> Maybe change the diagnostic to:
> 
> "defaulted definition of  cannot be declared 
> %select{constexpr|consteval}1 because its implicit definition would not be 
> constexpr"
> 
> or something like that?
> 
I don't think the change here has really addressed the problem. We now say:

`error: defaulted definition of  is not consteval`

in this case, which doesn't explain what's wrong, and appears to directly 
contradict what the user wrote. The problem is that the implicit definition 
would not be constexpr, so we should say that.


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

https://reviews.llvm.org/D61790



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


[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

2019-06-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked 2 inline comments as done.
rnk added a comment.

In D62975#1533019 , @inglorion wrote:

> Can you clarify "which will usually result in a linker error"? E.g. an 
> example of link.exe rejecting the object file or the wrong function being 
> called.


Their linker gives an error, but then it tries to be helpful:

  $ cat b.c
  struct Foo {int x; };
  int __fastcall bar(struct Foo o) { return o.x + 42; }
  extern int (__fastcall *fp)(struct Foo);
  int main() {
struct Foo o = { 13 };
return (*fp)(o);
  }
  
  $ cat t.c
  struct Foo;
  int __fastcall bar(struct Foo o);
  int (__fastcall *fp)(struct Foo) = 
  
  $cl -c t.c b.c && link t.obj b.obj -out:t.exe
  Microsoft (R) C/C++ Optimizing Compiler Version 19.20.27508.1 for x86
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  t.c
  b.c
  Generating Code...
  Microsoft (R) Incremental Linker Version 14.20.27508.1
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  t.obj : error LNK2001: unresolved external symbol @bar@0
Hint on symbols that are defined and could potentially match:
  @bar@4
  t.exe : fatal error LNK1120: 1 unresolved externals



> The reason I ask is that if we can be sure at compile time that the resulting 
> code will not work or do the wrong thing, I think giving an error is 
> appropriate. But if that isn't the case, we would be rejecting code that 
> cl.exe accepts and we might want to make it a Warning instead.

I guess the only reason I made it a hard error is that we'd have to teach 
codegen to tolerate incomplete types if we make it a warning that can be 
bypassed. But that might be worth doing anyway. We'd just do what they do, 
mangle to `@0`.




Comment at: clang/lib/Sema/SemaExpr.cpp:14801
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))

inglorion wrote:
> Do we need those checks or would it be enough to just check the calling 
> convention?
> 
> Also, I think s/Do nothing/return false/
I think you can use the calling conventions on non-Windows, but you don't get 
the mangling, so I think this should return false. Non-x86 architectures 
probably ignore the __fastcall qualifier with a warning, so we don't need the 
arch check.



Comment at: clang/lib/Sema/SemaExpr.cpp:14813
+  default:
+break;
+  }

inglorion wrote:
> You can just return false here.
I did it this way in an attempt to avoid worrying about the possibility of 
compilers that warn about falling off the end of the function. Such compilers 
used to exist, I don't know if we still support them, but I didn't want to find 
out, so I arranged it this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62975



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


[PATCH] D62970: [clang-doc] De-duplicate comments

2019-06-06 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

Actually if we can make Description a hash set that would be best. I think 
using std::set would require an awkward < operator to be defined. There should 
be standard ways of getting hashes out of all the string and vector types and 
you can use hash_combine to combine all of these into a single hash.


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

https://reviews.llvm.org/D62970



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


[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

2019-06-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:122
+  Warns if a function or method is called with default arguments.
+  This was previously done by fuchsia-default-arguments check, which has been
+  removed.

Please highlight fuchsia-default-arguments with single back-ticks.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:129
+  Warns if a function or method is declared with default parameters.
+  This was previously done by fuchsia-default-arguments check, which has been
+  removed.

Please highlight fuchsia-default-arguments with single back-ticks.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:176
+  Warnings of function or method calls and declarations with default arguments
+  were moved to fuchsia-default-arguments-calls and
+  fuchsia-default-arguments-declarations checks respectively.

Please add links to fuchsia-default-arguments-calls and 
fuchsia-default-arguments-declarations.


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

https://reviews.llvm.org/D62437



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


[PATCH] D62970: [clang-doc] De-duplicate comments

2019-06-06 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.cpp:124
+  for (auto  : Other.Description) {
+bool IsCommentUnique = std::find(Description.begin(), Description.end(),
+ Comment) == Description.end();

juliehockett wrote:
> ```if (std::find(Description.begin(), Description.end(), Comment) == 
> Description.end())
>   Description.emplace_back(std::move(Comment));```
Instead of deduping like this can we just make Description a set?



Comment at: clang-tools-extra/clang-doc/Representation.h:60-65
+for (unsigned I = 0; I < Children.size(); ++I) {
+  if (!(*Children[I] == *Other.Children[I]))
+return false;
+}
+return true;
+  }

You can do this using llvm::make_pointer_range/llvm::pointer_iterator and 
std::equal


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

https://reviews.llvm.org/D62970



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


[PATCH] D62855: Implementation of auto type expansion.

2019-06-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry about the delay here. Happy to chat offline if I'm being confusing.




Comment at: clang-tools-extra/clangd/AST.cpp:172
 
+namespace {
+/// Computes the deduced type at a given location by visiting the relevant

It looks like this has been moved from somewhere (at least code looks familiar) 
but isn't deleted anywhere. (The code in XRefs is touched but doesn't seem to 
use this). Is there a reason we can't reuse one copy?





Comment at: clang-tools-extra/clangd/AST.h:72
+// TODO: add documentation
+llvm::Optional getDeductedType(SourceLocation SearchedLocation, 
ASTContext );
+

nit: deduced
(also docs! In particular the fact that the SearchedLocation should point to 
exactly an `auto`, `decltype` etc token)



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp:76
+std::string
+ExpandAutoType::shortenNamespace(const llvm::StringRef ,
+ const llvm::StringRef ) {

This is nice. We might want to move it to SourceCode.h, somewhere near 
SplitQualifiedName. Then we'd generally unit test all the cases in 
SourceCodeTests, and we only have to smoke test the shortening here.

One limitation is that it only handles shortening the qualifier on the name 
itself, but not other parts of a printed type. e.g. `namespace ns { struct S(); 
auto* x = new std::vector(); }` will expand to `std::vector`, not 
`std::vector`. To fully solve this we may want to modify PrintingPolicy at 
some point, though we could probably get a long way by searching for chunks of 
text inside printed types that look like qualified names.

I think it might more clearly communicate the limitations if this function 
operated on the scope part only, e.g. `printScope("clang::clangd::", "clang::") 
--> "clangd::"`.

In the general case, `CurrentNamespace` should be a vector, because there can 
be others (introduced by using-declarations). Fine to leave this as a fixme if 
it's hard to do here, though.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp:76
+std::string
+ExpandAutoType::shortenNamespace(const llvm::StringRef ,
+ const llvm::StringRef ) {

sammccall wrote:
> This is nice. We might want to move it to SourceCode.h, somewhere near 
> SplitQualifiedName. Then we'd generally unit test all the cases in 
> SourceCodeTests, and we only have to smoke test the shortening here.
> 
> One limitation is that it only handles shortening the qualifier on the name 
> itself, but not other parts of a printed type. e.g. `namespace ns { struct 
> S(); auto* x = new std::vector(); }` will expand to `std::vector`, 
> not `std::vector`. To fully solve this we may want to modify 
> PrintingPolicy at some point, though we could probably get a long way by 
> searching for chunks of text inside printed types that look like qualified 
> names.
> 
> I think it might more clearly communicate the limitations if this function 
> operated on the scope part only, e.g. `printScope("clang::clangd::", 
> "clang::") --> "clangd::"`.
> 
> In the general case, `CurrentNamespace` should be a vector, because there can 
> be others (introduced by using-declarations). Fine to leave this as a fixme 
> if it's hard to do here, though.
StringRefs are passed by value. They're just shorthand for a char* + length.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.h:1
+//===--- Tweak.h -*- 
C++-*-===//
+//

update filename

(these headers are so silly :-()



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.h:21
+///std::string x = Something();
+///^^^
+class ExpandAutoType : public Tweak {

Maybe add a FIXME to handle decltype here too? It's pretty similar (though 
rarer).



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:221
 
+TEST(TweakTest, ExpandAutoType) {
+  llvm::StringLiteral ID = "ExpandAutoType";

can you add testcases for:
 - unknown types in a template `template  void x() { auto y = 
T::z(); }`
 - broken code `auto x = doesnt_exist();`
 - lambda `auto x = []{};`
 - inline/anon namespace: `inline namespace x { namespace { struct S; } } auto 
y = S();` should insert "S" not "x::S" or "(anonymous)::S".
 - local class: `namespace x { void y() { struct S{}; auto z = S(); } }` 
(should insert "S")

(it's ok if the behavior is bad in these cases, we can fix that later. Ideal in 
first 3 is no change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62855



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


[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

2019-06-06 Thread Diego Astiazarán via Phabricator via cfe-commits
DiegoAstiazaran updated this revision to Diff 203447.
DiegoAstiazaran added a comment.

Complete documentation in ReleaseNotes


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

https://reviews.llvm.org/D62437

Files:
  clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.h
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCheck.cpp
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCheck.h
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp
  clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h
  clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/fuchsia-default-arguments-calls.rst
  
clang-tools-extra/docs/clang-tidy/checks/fuchsia-default-arguments-declarations.rst
  clang-tools-extra/docs/clang-tidy/checks/fuchsia-default-arguments.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-calls.cpp
  clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
  clang-tools-extra/test/clang-tidy/fuchsia-default-arguments.cpp

Index: clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
===
--- clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
+++ clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp
@@ -1,26 +1,15 @@
-// RUN: %check_clang_tidy %s fuchsia-default-arguments %t
+// RUN: %check_clang_tidy %s fuchsia-default-arguments-declarations %t
 
 int foo(int value = 5) { return value; }
-// CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
 // CHECK-FIXES: int foo(int value) { return value; }
 
-int f() {
-  foo();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: [[@LINE-7]]:9: note: default parameter was declared here
-}
-
 int bar(int value) { return value; }
 
-int n() {
-  foo(0);
-  bar(0);
-}
-
 class Baz {
 public:
   int a(int value = 5) { return value; }
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+  // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
   // CHECK-FIXES: int a(int value) { return value; }
 
   int b(int value) { return value; }
@@ -29,7 +18,7 @@
 class Foo {
   // Fix should be suggested in declaration
   int a(int value = 53);
-  // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+  // CHECK-NOTES: [[@LINE-1]]:9: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
   // CHECK-FIXES: int a(int value);
 };
 
@@ -40,7 +29,7 @@
 
 // Elided functions
 void f(int = 5) {};
-// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
 // CHECK-FIXES: void f(int) {};
 
 void g(int) {};
@@ -49,12 +38,12 @@
 #define D(val) = val
 
 void h(int i D(5));
-// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
 // CHECK-FIXES-NOT: void h(int i);
 
 void x(int i);
 void x(int i = 12);
-// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:8: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
 // CHECK-FIXES: void x(int i);
 
 void x(int i) {}
@@ -64,17 +53,5 @@
 };
 
 void S::x(int i = 12) {}
-// CHECK-NOTES: [[@LINE-1]]:11: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments]
+// CHECK-NOTES: [[@LINE-1]]:11: warning: declaring a parameter with a default argument is disallowed [fuchsia-default-arguments-declarations]
 // CHECK-FIXES: void S::x(int i) {}
-
-int main() {
-  S s;
-  s.x();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: 

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

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

By the way, much to my surprise, this didn't start diagnosing the loop i 
expected it to start diagnosing:
https://godbolt.org/z/lsJTSS
This is expected?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827



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


[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

2019-06-06 Thread Lingda Li via Phabricator via cfe-commits
lildmh updated this revision to Diff 203438.
lildmh added a comment.

Use __tgt instead of __kmpc for mapper runtime function names


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

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,418 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass constructing objects to see inlined calls

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

If I run `scan-build` on LLVM this is the only non-bypassed case since the 
first diff: F9091851: non-bypassed.html 

I think it is working well.


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

https://reviews.llvm.org/D62926



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


[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

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

Suppressed example: F9091784: return-non-null.html 



Repository:
  rC Clang

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

https://reviews.llvm.org/D62978



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


[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

2019-06-06 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, 
Szelethus.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.
Charusso added a comment.

Suppressed example: F9091784: return-non-null.html 



If we have a value at the return side do not write out it is being null.


Repository:
  rC Clang

https://reviews.llvm.org/D62978

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/inlining/false-positive-suppression.cpp


Index: clang/test/Analysis/inlining/false-positive-suppression.cpp
===
--- clang/test/Analysis/inlining/false-positive-suppression.cpp
+++ clang/test/Analysis/inlining/false-positive-suppression.cpp
@@ -1,5 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
suppress-null-return-paths=false -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED=1 \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
 
 namespace rdar12676053 {
   // Delta-reduced from a preprocessed file.
@@ -85,7 +93,10 @@
 
 int * = m.getValue(i);
 box2 = 0;
-*box2 = 1; // expected-warning {{Dereference of null pointer}}
+*box2 = 1;
+#ifndef SUPPRESSED
+// expected-warning@-2 {{Dereference of null pointer}}
+#endif
   }
 
   SomeClass *() {
Index: clang/test/Analysis/diagnostics/find_last_store.c
===
--- clang/test/Analysis/diagnostics/find_last_store.c
+++ clang/test/Analysis/diagnostics/find_last_store.c
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text 
-verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -analyzer-output=text -verify %s
+
+// FIXME: Make it work with suppress-null-return-paths=true.
+
 typedef struct { float b; } c;
 void *a();
 void *d() {
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -827,8 +827,7 @@
   /// the statement is a call that was inlined, we add the visitor to the
   /// bug report, so it can print a note later.
   static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
-BugReport ,
-bool InEnableNullFPSuppression) {
+BugReport , bool IsNullFPSuppression) {
 if (!CallEvent::isCallStmt(S))
   return;
 
@@ -877,11 +876,11 @@
 // See if the return value is NULL. If so, suppress the report.
 AnalyzerOptions  = State->getAnalysisManager().options;
 
+// If we have a value at the return side do not write out it is being null.
 bool EnableNullFPSuppression = false;
-if (InEnableNullFPSuppression &&
-Options.ShouldSuppressNullReturnPaths)
+if (IsNullFPSuppression && Options.ShouldSuppressNullReturnPaths)
   if (Optional RetLoc = RetVal.getAs())
-EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
+EnableNullFPSuppression = true;
 
 BR.markInteresting(CalleeContext);
 BR.addVisitor(llvm::make_unique(CalleeContext,


Index: clang/test/Analysis/inlining/false-positive-suppression.cpp
===
--- clang/test/Analysis/inlining/false-positive-suppression.cpp
+++ clang/test/Analysis/inlining/false-positive-suppression.cpp
@@ -1,5 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED=1 \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
 
 namespace rdar12676053 {
   // Delta-reduced from a preprocessed file.
@@ -85,7 +93,10 @@
 
 int * = m.getValue(i);
 box2 = 0;
-*box2 = 1; // expected-warning {{Dereference of null pointer}}
+*box2 = 1;
+#ifndef SUPPRESSED
+// expected-warning@-2 {{Dereference of null pointer}}
+#endif
   }
 
   SomeClass *() {
Index: clang/test/Analysis/diagnostics/find_last_store.c

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

2019-06-06 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

In D61446#1532848 , @Meinersbur wrote:

> Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in 
> opt/bugpoint/clang by adding Polly.cpp as a source file or object library to 
> the executables. This would guarantee that its static initializer is called 
> without dynamic library.


That's what I tried to do when always adding Polly.cpp to PollyCore, but it 
doesn't seem to be enough, I still need to invstigate why (it is actually 
enough when using BUILD_SHARED_LIBS=ON)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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


[PATCH] D62808: [clangd] Print the ignored variant symbols

2019-06-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/include-mapping/gen_std.py:119
+( a list of tuple (symbol_name, relative_path_to_symbol_page),
+  a list of tuple (variant_symbol_name, caption_text) )
   """

this is a significantly more complicated signature - do we need the extra 
precision in the log message vs e.g. `Ignored variant of symbol abs from 
path/to/complex.html`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62808



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


[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

2019-06-06 Thread Bob Haarman via Phabricator via cfe-commits
inglorion added a comment.

Can you clarify "which will usually result in a linker error"? E.g. an example 
of link.exe rejecting the object file or the wrong function being called. The 
reason I ask is that if we can be sure at compile time that the resulting code 
will not work or do the wrong thing, I think giving an error is appropriate. 
But if that isn't the case, we would be rejecting code that cl.exe accepts and 
we might want to make it a Warning instead.




Comment at: clang/lib/Sema/SemaExpr.cpp:14801
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))

Do we need those checks or would it be enough to just check the calling 
convention?

Also, I think s/Do nothing/return false/



Comment at: clang/lib/Sema/SemaExpr.cpp:14813
+  default:
+break;
+  }

You can just return false here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62975



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


[PATCH] D62804: [clangd] Enable extraction of system includes from custom toolchains

2019-06-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Implementation looks good. I can't see a better way to solve this problem, it's 
just a bit unfortunate to have a sophisticated solution but not be able to turn 
it on by default.

I think naming is important here: it's a fairly complicated feature that (I 
suppose) relatively few will use, so having an unambiguous way to refer to it 
e.g. in docs will reduce the cost/confusion.
I suggested "QueryDriver" below, but we might be able to come up with something 
better offline.




Comment at: clang-tools-extra/clangd/ClangdServer.h:127
+
+/// If set clangd will execute compiler drivers matching the following 
regex
+/// to fetch system include path.

not a regex



Comment at: clang-tools-extra/clangd/ClangdServer.h:127
+
+/// If set clangd will execute compiler drivers matching the following 
regex
+/// to fetch system include path.

sammccall wrote:
> not a regex
I think this probably wants to be a vector (comma-separated on 
command-line)?

Clangd-level settings aren't easy to customize per-project.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:65
 
+std::vector parseDriverOutput(llvm::StringRef Output) {
+  std::vector SystemIncludes;

Can we move the implementation out to a separate file?
It's pretty logically separate, and about as much code as everything else 
together.
It could also use a file-level comment describing how the scheme works, and 
also the reasons it's not always on (security, and not all argv0's are 
gcc-compatible)
(Sharing the same header seems fine)



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:71
+  Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  bool FoundStart = false;
+  for (llvm::StringRef Line : Lines) {

I'd consider `std::search` for the delimiters explicitly - I think it's clearer 
what happens when you find one but not the other.

(And you probably want to log an error and do nothing unless you find both)



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:82
+
+if (!llvm::sys::fs::exists(Line))
+  elog("Parsed non-existing system include: {0}", Line);

Is this check important? (What happens if non-existent dirs are on the include 
path?)

If it is, maybe we should pass in the VFS (factory?) here.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:90
+
+std::vector extractSystemIncludes(PathRef Driver, PathRef File) {
+  trace::Span Tracer("Extract system includes");

some explanation of what this is doing?

e.g. "Many compilers have default system includes... these are known by the 
driver and passed to cc1 ... gcc and compilers with compatible CLI can dump the 
cc1 invocation information with this syntax... we do so and parse the result."



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:96
+  llvm::SmallString<128> OutputPath;
+  const auto EC = llvm::sys::fs::createTemporaryFile("system-includes",
+ "clangd", OutputPath);

nit: please don't bother with const on locals (style is debatable, but it's 
hardly used anywhere and adds little value unless used consistently)



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:96
+  llvm::SmallString<128> OutputPath;
+  const auto EC = llvm::sys::fs::createTemporaryFile("system-includes",
+ "clangd", OutputPath);

sammccall wrote:
> nit: please don't bother with const on locals (style is debatable, but it's 
> hardly used anywhere and adds little value unless used consistently)
we need to remove this file at some point



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:99
+  if (EC) {
+elog("Couldn't create temporary file: {0}", EC.message());
+return {};

log messages should contain more context, e.g. "Driver flags extraction: failed 
to create temporary file"



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:108
+  // Should we also preserve flags like "-sysroot", "-nostdinc" ?
+  const llvm::StringRef Args[] = {"-E", "-x", driver::types::getTypeName(Type),
+  "-", "-v"};

this will crash if the lookup didn't provide a "real" type



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:112
+  const int RC =
+  llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None, Redirects);
+  if (RC) {

should we check the driver exists before executing it? the main advantage would 
be that we avoid logging this as if it were an error (or we do so with a better 
error message)




[PATCH] D62971: [HIP] Remove the assertion on match between host/device names.

2019-06-06 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

I took this back. I fab a case where anonymous type IDs mismatch between the 
device-side name between host-compilation and device-compilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62971



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


[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

2019-06-06 Thread Alex Strelnikov via Phabricator via cfe-commits
astrelni created this revision.
astrelni added reviewers: alexfh, hokein, aaron.ballman, JonasToth, EricWF.
astrelni added a project: clang-tools-extra.
Herald added subscribers: llvm-commits, cfe-commits, xazax.hun, mgorny.
Herald added projects: clang, LLVM.

Introduce a new check to upgrade user code based on API changes in Googletest.

The check finds uses of old Googletest APIs with "case" in their name and 
replaces them with the new APIs named with "suite".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62977

Files:
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/google-upgrade-googletest-case.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest-typed-test.h
  clang-tools-extra/test/clang-tidy/Inputs/gtest/gtest.h
  clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/google/BUILD.gn
@@ -28,6 +28,7 @@
 "OverloadedUnaryAndCheck.cpp",
 "TodoCommentCheck.cpp",
 "UnnamedNamespaceInHeaderCheck.cpp",
+"UpgradeGoogletestCaseCheck.cpp",
 "UsingNamespaceDirectiveCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-upgrade-googletest-case.cpp
@@ -0,0 +1,991 @@
+// RUN: %check_clang_tidy %s google-upgrade-googletest-case %t -- -- -I%S/Inputs
+
+#include "gtest/gtest.h"
+
+// 
+// Macros
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: TYPED_TEST_SUITE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: REGISTER_TYPED_TEST_SUITE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+// CHECK-FIXES: INSTANTIATE_TYPED_TEST_SUITE_P(FooPrefix, FooTest, FooTypes);
+
+#ifdef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE TYPED_TEST_SUITE
+#endif
+
+#ifdef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define TYPED_TEST_CASE_P TYPED_TEST_SUITE_P
+#endif
+
+#ifdef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef REGISTER_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define REGISTER_TYPED_TEST_CASE_P REGISTER_TYPED_TEST_SUITE_P
+#endif
+
+#ifdef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:2: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#undef INSTANTIATE_TYPED_TEST_CASE_P
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: Googletest APIs named with 'case' are deprecated; use equivalent APIs named with 'suite'
+#define INSTANTIATE_TYPED_TEST_CASE_P INSTANTIATE_TYPED_TEST_SUITE_P
+#endif
+
+TYPED_TEST_CASE(FooTest, FooTypes);
+TYPED_TEST_CASE_P(FooTest);
+REGISTER_TYPED_TEST_CASE_P(FooTest, FooTestName);
+INSTANTIATE_TYPED_TEST_CASE_P(FooPrefix, FooTest, FooTypes);
+
+// 
+// 

[PATCH] D37813: clang-format: better handle namespace macros

2019-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362740: clang-format: better handle namespace macros 
(authored by Typz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D37813

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/FormatTokenLexer.cpp
  cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/TokenAnnotator.h
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1870,9 +1870,117 @@
Style));
 }
 
+TEST_F(FormatTest, NamespaceMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  verifyFormat("TESTSUITE(A) {\n"
+   "int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+
+  verifyFormat("TESTSUITE(A, B) {\n"
+   "int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+
+  // Properly indent according to NamespaceIndentation style
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("TESTSUITE(A) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("TESTSUITE(A) {\n"
+   "  namespace B {\n"
+   "int foo();\n"
+   "  } // namespace B\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("namespace A {\n"
+   "  TESTSUITE(B) {\n"
+   "int foo();\n"
+   "  } // TESTSUITE(B)\n"
+   "} // namespace A",
+   Style);
+
+  Style.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("TESTSUITE(A) {\n"
+   "TESTSUITE(B) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(B)\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("TESTSUITE(A) {\n"
+   "namespace B {\n"
+   "  int foo();\n"
+   "} // namespace B\n"
+   "} // TESTSUITE(A)",
+   Style);
+  verifyFormat("namespace A {\n"
+   "TESTSUITE(B) {\n"
+   "  int foo();\n"
+   "} // TESTSUITE(B)\n"
+   "} // namespace A",
+   Style);
+
+  // Properly merge namespace-macros blocks in CompactNamespaces mode
+  Style.NamespaceIndentation = FormatStyle::NI_None;
+  Style.CompactNamespaces = true;
+  verifyFormat("TESTSUITE(A) { TESTSUITE(B) {\n"
+   "}} // TESTSUITE(A::B)",
+   Style);
+
+  EXPECT_EQ("TESTSUITE(out) { TESTSUITE(in) {\n"
+"}} // TESTSUITE(out::in)",
+format("TESTSUITE(out) {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+
+  EXPECT_EQ("TESTSUITE(out) { TESTSUITE(in) {\n"
+"}} // TESTSUITE(out::in)",
+format("TESTSUITE(out) {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+
+  // Do not merge different namespaces/macros
+  EXPECT_EQ("namespace out {\n"
+"TESTSUITE(in) {\n"
+"} // TESTSUITE(in)\n"
+"} // namespace out",
+format("namespace out {\n"
+   "TESTSUITE(in) {\n"
+   "} // TESTSUITE(in)\n"
+   "} // namespace out",
+   Style));
+  EXPECT_EQ("TESTSUITE(out) {\n"
+"namespace in {\n"
+"} // namespace in\n"
+"} // TESTSUITE(out)",
+format("TESTSUITE(out) {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // TESTSUITE(out)",
+   Style));
+  Style.NamespaceMacros.push_back("FOOBAR");
+  EXPECT_EQ("TESTSUITE(out) {\n"
+"FOOBAR(in) {\n"
+"} // FOOBAR(in)\n"
+"} // TESTSUITE(out)",
+format("TESTSUITE(out) {\n"
+   "FOOBAR(in) {\n"
+   "} // FOOBAR(in)\n"
+   "} // TESTSUITE(out)",
+   Style));
+}
+
 TEST_F(FormatTest, FormatsCompactNamespaces) {
   FormatStyle Style = getLLVMStyle();
   Style.CompactNamespaces = true;
+  

r362740 - clang-format: better handle namespace macros

2019-06-06 Thread Francois Ferrand via cfe-commits
Author: typz
Date: Thu Jun  6 13:06:23 2019
New Revision: 362740

URL: http://llvm.org/viewvc/llvm-project?rev=362740=rev
Log:
clang-format: better handle namespace macros

Summary:
Other macros are used to declare namespaces, and should thus be handled
similarly. This is the case for crpcut's TESTSUITE macro, or for
unittest-cpp's SUITE macro:

  TESTSUITE(Foo) {
  TEST(MyFirstTest) {
assert(0);
  }
  } // TESTSUITE(Foo)

This patch deals with this cases by introducing a new option to specify
lists of namespace macros. Internally, it re-uses the system already in
place for foreach and statement macros, to ensure there is no impact on
performance.

Reviewers: krasimir, djasper, klimek

Reviewed By: klimek

Subscribers: acoomans, cfe-commits, klimek

Tags: #clang

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=362740=362739=362740=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Jun  6 13:06:23 2019
@@ -1782,6 +1782,19 @@ the configuration (without a prefix: ``A
 
 
 
+**NamespaceMacros** (``std::vector``)
+  A vector of macros which are used to open namespace blocks.
+
+  These are expected to be macros of the form:
+
+  .. code-block:: c++
+
+NAMESPACE(, ...) {
+  
+}
+
+  For example: TESTSUITE
+
 **ObjCBinPackProtocolList** (``BinPackStyle``)
   Controls bin-packing Objective-C protocol conformance list
   items into as few lines as possible when they go over ``ColumnLimit``.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=362740=362739=362740=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Thu Jun  6 13:06:23 2019
@@ -1195,6 +1195,18 @@ struct FormatStyle {
   /// For example: Q_UNUSED
   std::vector StatementMacros;
 
+  /// A vector of macros which are used to open namespace blocks.
+  ///
+  /// These are expected to be macros of the form:
+  /// \code
+  ///   NAMESPACE(, ...) {
+  /// 
+  ///   }
+  /// \endcode
+  ///
+  /// For example: TESTSUITE
+  std::vector NamespaceMacros;
+
   tooling::IncludeStyle IncludeStyle;
 
   /// Indent case labels one level from the switch statement.
@@ -1942,6 +1954,7 @@ struct FormatStyle {
MacroBlockEnd == R.MacroBlockEnd &&
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
NamespaceIndentation == R.NamespaceIndentation &&
+   NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=362740=362739=362740=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Jun  6 13:06:23 2019
@@ -455,6 +455,7 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=362740=362739=362740=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Thu Jun  6 13:06:23 2019
@@ -71,6 +71,7 @@ namespace format {
   TYPE(LineComment)
\
   TYPE(MacroBlockBegin)
\
   TYPE(MacroBlockEnd)  
\
+  TYPE(NamespaceMacro) 
\
   TYPE(ObjCBlockLBrace)
\
   TYPE(ObjCBlockLParen)
\
   TYPE(ObjCDecl)   
\
@@ -531,8 +532,10 @@ struct FormatToken {
 // Detect "(inline|export)? 

[PATCH] D37813: clang-format: better handle namespace macros

2019-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 203425.
Typz added a comment.

Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/TokenAnnotator.h
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -52,6 +52,7 @@
 "int i;\n"
 "int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "int j;\n"
@@ -248,6 +249,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"int i;\n"
+"int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"int i;\n"
+"int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "int i;\n"
@@ -380,6 +460,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  

[PATCH] D62971: [HIP] Remove the assertion on match between host/device names.

2019-06-06 Thread Michael Liao via Phabricator via cfe-commits
hliao marked 2 inline comments as done.
hliao added inline comments.



Comment at: clang/test/CodeGenCUDA/unnamed-types.cu:5
+
+// CHECK: @0 = private unnamed_addr constant [40 x i8] 
c"_Z2k0IZZ2f1PfENK3$_0clES0_EUlfE_EvS0_T_\00"
+

device-side mangled name, notice that `$_0` refers to the unnamed closure in f1.



Comment at: clang/test/CodeGenCUDA/unnamed-types.cu:24
+// CHECK: @__hip_register_globals
+// CHECK: 
__hipRegisterFunction{{.*}}_Z2k0IZZ2f1PfENK3$_1clES0_EUlfE_EvS0_T_{{.*}}@0

the registration of host-side stub function to the device-side function name, 
which is defined in `@0`. Notice that the host-side stub function has `$_1`, 
which refers to the closure in f1 as there's another closure (host-only) in f0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62971



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


[PATCH] D62971: [HIP] Remove the assertion on match between host/device names.

2019-06-06 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

a little explanation of the test case and what's issue is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62971



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


[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

2019-06-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: thakis, rsmith, hans.
Herald added a project: clang.

Functions using stdcall, fastcall, or vectorcall with C linkage mangle
in the size of the parameter pack. Calculating the size of the pack
requires the parameter types to complete, which may require template
instantiation.

Previously, we would crash during IRgen when requesting the size of
incomplete or uninstantiated types, as in this reduced example:

  struct Foo;
  void __fastcall bar(struct Foo o);
  void (__fastcall *fp)(struct Foo) = 

Reported in Chromium here: https://crbug.com/971245


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62975

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/calling-conv-complete-params.cpp

Index: clang/test/SemaCXX/calling-conv-complete-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/calling-conv-complete-params.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -fms-extensions -verify -triple i686-pc-win32 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -fms-extensions -verify -triple x86_64-pc-win32 %s
+
+struct Foo; // expected-note 1+ {{forward declaration of 'Foo'}}
+
+void __stdcall fwd_std(struct Foo p);
+#ifdef _M_IX86
+// expected-error@+2 {{parameter 'p' must have a complete type to use function 'fwd_std' with the stdcall calling convention}}
+#endif
+void (__stdcall *fp_fwd_std)(struct Foo) = _std;
+
+void __fastcall fwd_fast(struct Foo p);
+#ifdef _M_IX86
+// expected-error@+2 {{parameter 'p' must have a complete type to use function 'fwd_fast' with the fastcall calling convention}}
+#endif
+void (__fastcall *fp_fwd_fast)(struct Foo) = _fast;
+
+void __vectorcall fwd_vector(struct Foo p);
+// expected-error@+1 {{parameter 'p' must have a complete type to use function 'fwd_vector' with the vectorcall calling convention}}
+void (__vectorcall *fp_fwd_vector)(struct Foo) = _vector;
+
+template  struct TemplateWrapper {
+  T field; // expected-error {{field has incomplete type 'Foo'}}
+};
+
+void __vectorcall tpl_ok(TemplateWrapper p);
+void(__vectorcall *fp_tpl_ok)(TemplateWrapper) = _ok;
+
+void __vectorcall tpl_fast(TemplateWrapper p);
+void(__vectorcall *fp_tpl_fast)(TemplateWrapper) = _fast; // expected-note {{requested here}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14793,6 +14793,74 @@
   llvm_unreachable("Invalid context");
 }
 
+/// Return true if this function has a calling convention that requires mangling
+/// in the size of the parameter pack.
+static bool funcHasParameterSizeMangling(Sema , FunctionDecl *FD) {
+  // Do nothing on non-Windows, non-x86/x64 platforms.
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))
+return false;
+
+  // Stdcall, fastcall, and vectorcall need this special treatment.
+  CallingConv CC = FD->getType()->castAs()->getCallConv();
+  switch (CC) {
+  case CC_X86StdCall:
+  case CC_X86FastCall:
+  case CC_X86VectorCall:
+return true;
+  default:
+break;
+  }
+  return false;
+}
+
+/// Require that all of the parameter types of function be complete. Normally,
+/// parameter types are only required to be complete when a function is called
+/// or defined, but to mangle functions with certain calling conventions, the
+/// mangler needs to know the size of the parameter list. In this situation,
+/// MSVC doesn't emit an error or instantiate templates. Instead, MSVC mangles
+/// the function as _foo@0, i.e. zero bytes of parameters, which will usually
+/// result in a linker error. Clang doesn't implement this behavior, and instead
+/// attempts to error at compile time.
+static void CheckCompleteParameterTypesForMangler(Sema , FunctionDecl *FD,
+  SourceLocation Loc) {
+  class ParamIncompleteTypeDiagnoser : public Sema::TypeDiagnoser {
+FunctionDecl *FD;
+ParmVarDecl *Param;
+
+  public:
+ParamIncompleteTypeDiagnoser(FunctionDecl *FD, ParmVarDecl *Param)
+: FD(FD), Param(Param) {}
+
+void diagnose(Sema , SourceLocation Loc, QualType T) override {
+  CallingConv CC = FD->getType()->castAs()->getCallConv();
+  StringRef CCName;
+  switch (CC) {
+  case CC_X86StdCall:
+CCName = "stdcall";
+break;
+  case CC_X86FastCall:
+CCName = "fastcall";
+break;
+  case CC_X86VectorCall:
+CCName = "vectorcall";
+break;
+  default:
+llvm_unreachable("CC does not need mangling");
+  }
+
+  S.Diag(Loc, diag::err_cconv_incomplete_param_type)
+  << Param->getDeclName() << FD->getDeclName() << CCName;
+}
+  };
+
+  for 

[PATCH] D62971: [HIP] Remove the assertion on match between host/device names.

2019-06-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

LGTM. It seems no reason to assume the mangled name to be same on host and 
device side once anonymous types are mangled differently in host and device 
code. On windows, kernel has totally different names on host and device side 
without issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62971



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


[PATCH] D62971: [HIP] Remove the assertion on match between host/device names.

2019-06-06 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added a reviewer: yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Under different ABIs, it's obvious that assertion is too strong. Even under 
the same ABI, once there are unnamed type not required to follow ODR rule, 
host- and device-side mangling may still get different names. As both the host- 
and device-side compilation always observe the same AST tree, even with 
different names, we still could associate the correct pairs, i.e., we don't use 
(mangled) names to linkage host- and device-side globals. There's no need to 
have this assertion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62971

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/test/CodeGenCUDA/unnamed-types.cu


Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -emit-llvm %s -o 
- | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: @0 = private unnamed_addr constant [40 x i8] 
c"_Z2k0IZZ2f1PfENK3$_0clES0_EUlfE_EvS0_T_\00"
+
+template 
+__global__ void k0(float *p, F f) {
+  p[0] = f(p[0]);
+}
+
+void f0(float *p) {
+  [](float *p) {
+*p = 1.f;
+  }(p);
+}
+
+void f1(float *p) {
+  [](float *p) {
+k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; });
+  }(p);
+}
+// CHECK: @__hip_register_globals
+// CHECK: 
__hipRegisterFunction{{.*}}_Z2k0IZZ2f1PfENK3$_1clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -217,11 +217,6 @@
 
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction ,
  FunctionArgList ) {
-  assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
- getDeviceSideName(CGF.CurFuncDecl) + ".stub" == CGF.CurFn->getName() 
||
- CGF.CGM.getContext().getTargetInfo().getCXXABI() !=
- CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI());
-
   EmittedKernels.push_back({CGF.CurFn, CGF.CurFuncDecl});
   if (CudaFeatureEnabled(CGM.getTarget().getSDKVersion(),
  CudaFeature::CUDA_USES_NEW_LAUNCH))


Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: @0 = private unnamed_addr constant [40 x i8] c"_Z2k0IZZ2f1PfENK3$_0clES0_EUlfE_EvS0_T_\00"
+
+template 
+__global__ void k0(float *p, F f) {
+  p[0] = f(p[0]);
+}
+
+void f0(float *p) {
+  [](float *p) {
+*p = 1.f;
+  }(p);
+}
+
+void f1(float *p) {
+  [](float *p) {
+k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; });
+  }(p);
+}
+// CHECK: @__hip_register_globals
+// CHECK: __hipRegisterFunction{{.*}}_Z2k0IZZ2f1PfENK3$_1clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -217,11 +217,6 @@
 
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction ,
  FunctionArgList ) {
-  assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
- getDeviceSideName(CGF.CurFuncDecl) + ".stub" == CGF.CurFn->getName() ||
- CGF.CGM.getContext().getTargetInfo().getCXXABI() !=
- CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI());
-
   EmittedKernels.push_back({CGF.CurFn, CGF.CurFuncDecl});
   if (CudaFeatureEnabled(CGM.getTarget().getSDKVersion(),
  CudaFeature::CUDA_USES_NEW_LAUNCH))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62965: [clang][HeaderSearch] Consider all path separators equal

2019-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362731: [clang][HeaderSearch] Consider all path separators 
equal (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62965?vs=203399=203409#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62965

Files:
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/unittests/Lex/HeaderSearchTest.cpp


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {


Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }
Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
===
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62970: [clang-doc] De-duplicate comments

2019-06-06 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett added a comment.

Please add a test case to `unittests/clang-doc/MergeTest.cpp`.




Comment at: clang-tools-extra/clang-doc/Representation.cpp:124-127
+bool IsCommentUnique = std::find(Description.begin(), Description.end(),
+ Comment) == Description.end();
+if (IsCommentUnique)
+  Description.emplace_back(std::move(Comment));

```if (std::find(Description.begin(), Description.end(), Comment) == 
Description.end())
  Description.emplace_back(std::move(Comment));```



Comment at: clang-tools-extra/clang-doc/Representation.h:51-59
+bool AreEqual =
+Kind == Other.Kind && Text == Other.Text && Name == Other.Name &&
+Direction == Other.Direction && ParamName == Other.ParamName &&
+CloseName == Other.CloseName && SelfClosing == Other.SelfClosing &&
+Explicit == Other.Explicit && AttrKeys == Other.AttrKeys &&
+AttrValues == Other.AttrValues && Args == Other.Args &&
+Children.size() == Other.Children.size();

Use `std::tie` here (see `Reference::operator==` below as an example).


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

https://reviews.llvm.org/D62970



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


r362731 - [clang][HeaderSearch] Consider all path separators equal

2019-06-06 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Thu Jun  6 11:49:16 2019
New Revision: 362731

URL: http://llvm.org/viewvc/llvm-project?rev=362731=rev
Log:
[clang][HeaderSearch] Consider all path separators equal

Reviewers: ilya-biryukov, sammccall

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=362731=362730=362731=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Jun  6 11:49:16 2019
@@ -1715,6 +1715,11 @@ std::string HeaderSearch::suggestPathToF
 break;
   }
 
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }

Modified: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/HeaderSearchTest.cpp?rev=362731=362730=362731=diff
==
--- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp (original)
+++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp Thu Jun  6 11:49:16 2019
@@ -98,6 +98,13 @@ TEST_F(HeaderSearchTest, BackSlash) {
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {


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


[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

2019-06-06 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in 
opt/bugpoint/clang by adding Polly.cpp as a source file or object library to 
the executables. This would guarantee that its static initializer is called 
without dynamic library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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


[PATCH] D62970: [clang-doc] De-duplicate comments

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

Test?


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

https://reviews.llvm.org/D62970



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


[PATCH] D62970: [clang-doc] De-duplicate comments

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

De-duplicate comments in reduce function.
When two files include the same header file, this file's content is mapped 
twice and comments are duplicated after the reduce stage.


https://reviews.llvm.org/D62970

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -47,6 +47,23 @@
   CommentInfo(CommentInfo ) = delete;
   CommentInfo(CommentInfo &) = default;
 
+  bool operator==(const CommentInfo ) const {
+bool AreEqual =
+Kind == Other.Kind && Text == Other.Text && Name == Other.Name &&
+Direction == Other.Direction && ParamName == Other.ParamName &&
+CloseName == Other.CloseName && SelfClosing == Other.SelfClosing &&
+Explicit == Other.Explicit && AttrKeys == Other.AttrKeys &&
+AttrValues == Other.AttrValues && Args == Other.Args &&
+Children.size() == Other.Children.size();
+if (!AreEqual)
+  return false;
+for (unsigned I = 0; I < Children.size(); ++I) {
+  if (!(*Children[I] == *Other.Children[I]))
+return false;
+}
+return true;
+  }
+
   SmallString<16>
   Kind; // Kind of comment (FullComment, ParagraphComment, TextComment,
 // InlineCommandComment, HTMLStartTagComment, HTMLEndTagComment,
Index: clang-tools-extra/clang-doc/Representation.cpp
===
--- clang-tools-extra/clang-doc/Representation.cpp
+++ clang-tools-extra/clang-doc/Representation.cpp
@@ -120,8 +120,12 @@
   if (Namespace.empty())
 Namespace = std::move(Other.Namespace);
   // Unconditionally extend the description, since each decl may have a 
comment.
-  std::move(Other.Description.begin(), Other.Description.end(),
-std::back_inserter(Description));
+  for (auto  : Other.Description) {
+bool IsCommentUnique = std::find(Description.begin(), Description.end(),
+ Comment) == Description.end();
+if (IsCommentUnique)
+  Description.emplace_back(std::move(Comment));
+  }
 }
 
 bool Info::mergeable(const Info ) {


Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -47,6 +47,23 @@
   CommentInfo(CommentInfo ) = delete;
   CommentInfo(CommentInfo &) = default;
 
+  bool operator==(const CommentInfo ) const {
+bool AreEqual =
+Kind == Other.Kind && Text == Other.Text && Name == Other.Name &&
+Direction == Other.Direction && ParamName == Other.ParamName &&
+CloseName == Other.CloseName && SelfClosing == Other.SelfClosing &&
+Explicit == Other.Explicit && AttrKeys == Other.AttrKeys &&
+AttrValues == Other.AttrValues && Args == Other.Args &&
+Children.size() == Other.Children.size();
+if (!AreEqual)
+  return false;
+for (unsigned I = 0; I < Children.size(); ++I) {
+  if (!(*Children[I] == *Other.Children[I]))
+return false;
+}
+return true;
+  }
+
   SmallString<16>
   Kind; // Kind of comment (FullComment, ParagraphComment, TextComment,
 // InlineCommandComment, HTMLStartTagComment, HTMLEndTagComment,
Index: clang-tools-extra/clang-doc/Representation.cpp
===
--- clang-tools-extra/clang-doc/Representation.cpp
+++ clang-tools-extra/clang-doc/Representation.cpp
@@ -120,8 +120,12 @@
   if (Namespace.empty())
 Namespace = std::move(Other.Namespace);
   // Unconditionally extend the description, since each decl may have a comment.
-  std::move(Other.Description.begin(), Other.Description.end(),
-std::back_inserter(Description));
+  for (auto  : Other.Description) {
+bool IsCommentUnique = std::find(Description.begin(), Description.end(),
+ Comment) == Description.end();
+if (IsCommentUnique)
+  Description.emplace_back(std::move(Comment));
+  }
 }
 
 bool Info::mergeable(const Info ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62839: [clangd] Index API and implementations for relations

2019-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.cpp:293
+  // This map is used to figure out where to store relations.
+  llvm::DenseMap IDsToFiles;
   for (const auto  : *Index.Symbols) {

nit: rename to `SymbolIDToFile`?



Comment at: clang-tools-extra/clangd/index/Background.cpp:329
+const auto FileIt = IDsToFiles.find(Rel.Subject);
+if (FileIt != IDsToFiles.end()) {
+  FileIt->second->Relations.insert();

nit: braces



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:201
+  std::vector AllRelations;
+  {
+for (const auto  : RelationSlabs) {

redundant scope



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:225
 std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
-std::move(RefsStorage), std::move(SymsStorage)),
+std::move(RelationSlabs), std::move(RefsStorage),
+std::move(SymsStorage)),

no need to pass `RelationSlabs` in here `AllRelations` are just value types.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:250
   Path, llvm::make_unique(std::move(Symbols)),
-  llvm::make_unique(), /*CountReferences=*/false);
+  llvm::make_unique(), llvm::make_unique(),
+  /*CountReferences=*/false);

I think we need to pass relationslab in here. Since we might miss relations 
like the following that are outside the main file:
```
class A {};
class B : public A {};
```
Would be glad if you could prove me right/wrong with a unittest as well.



Comment at: clang-tools-extra/clangd/index/Index.h:107
+  virtual void
+  relations(const SymbolID , index::SymbolRole Predicate,
+llvm::function_ref Callback) const = 0;

We might need additional options, like limiting number of results. Could you 
instead accept a `RelsRequest` object? You can check `RefsRequest` for a sample



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:22
   // Store Slab size before it is moved.
   const auto BackingDataSize = Slab.bytes() + Refs.bytes();
+  auto Data =

`+ Relations.bytes()`



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:24
+  auto Data =
+  std::make_tuple(std::move(Slab), std::move(Refs), std::move(Relations));
+  return llvm::make_unique(std::get<0>(Data), std::get<1>(Data),

no need to put `Relations` into `Data` they are just values, right?



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:94
+  auto Rels = Relations.lookup(Subject, Predicate);
+  for (const auto  : Rels) {
+Callback(R.Object);

nit: braces



Comment at: clang-tools-extra/clangd/index/MemIndex.h:29
   this->Refs.try_emplace(R.first, R.second.begin(), R.second.end());
+RelationSlab::Builder Builder;
+for (const Relation  : Relations)

why not just store `densemap<,arrayref>` ?



Comment at: clang-tools-extra/clangd/index/Merge.cpp:126
+llvm::function_ref Callback) const {
+  // Return results from both indexes but avoid duplicates.
+  llvm::DenseSet SeenObjects;

avoiding deduplicates is not enough, we also wouldn't want to report stale 
relations coming from static index.

Could you check the logic in `MergedIndex::refs`, and hopefully refactor it 
into a class to share between these two?



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:28
+RelationSlab Rels) {
   auto Size = Symbols.bytes() + Refs.bytes();
+  // There is no need to include "Rels" in Data because the relations are self-

`+rels.size()`



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:269
+  auto Rels = Relations.lookup(Subject, Predicate);
+  for (const auto  : Rels) {
+Callback(R.Object);

nit: braces



Comment at: clang-tools-extra/clangd/index/dex/Dex.h:110
   llvm::DenseMap> Refs;
+  RelationSlab Relations;
   std::shared_ptr KeepAlive; // poor man's move-only std::any

let's store `densemap<, arrayref>` here as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62839



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


[PATCH] D62965: [clang][HeaderSearch] Consider all path separators equal

2019-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 203399.
kadircet marked an inline comment as done.
kadircet added a comment.

- Update comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62965

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Consider all path separators equal.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62965: [clang][HeaderSearch] Consider all path separators equal

2019-06-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1718
 
+  // Ignore path separators.
+  if (NI->size() == 1 && DI->size() == 1 &&

I think "consider all path separators equal" as in the patch description would 
be much clearer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62965



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


[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

2019-06-06 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett added a comment.

Sorry for the delay!




Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp:24
+  const auto *S = Result.Nodes.getNodeAs("stmt");
+  if (S == nullptr)
+return;

Just `if (!S)` should be sufficient here.



Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.cpp:25
+  const auto *D = Result.Nodes.getNodeAs("decl");
+  if (D == nullptr)
+return;

Same here for the `if` as above.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:118-122
+- New :doc:`fuchsia-default-arguments-declarations
+  ` check.
+
+  Warns if a function or method is declared with default parameters.
+

Also include a line about the addition of fuchsia-default-arguments-calls and 
the removal of fuchsia-default-arguments.



Comment at: 
clang-tools-extra/test/clang-tidy/fuchsia-default-arguments-declarations.cpp:59-60
 
 int main() {
-  S s;
-  s.x();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a 
default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: [[@LINE-8]]:11: note: default parameter was declared here
-  // CHECK-NEXT: void S::x(int i = 12) {}
-  x();
-  // CHECK-NOTES: [[@LINE-1]]:3: warning: calling a function that uses a 
default argument is disallowed [fuchsia-default-arguments]
-  // CHECK-NOTES: [[@LINE-18]]:8: note: default parameter was declared here
-  // CHECK-NEXT: void x(int i = 12);
 }

Remove this


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

https://reviews.llvm.org/D62437



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


[PATCH] D62965: [clang][HeaderSearch] Consider all path separators equal

2019-06-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added reviewers: ilya-biryukov, sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62965

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Ignore path separators.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -98,6 +98,13 @@
/*WorkingDir=*/""),
 "z/t");
 }
+
+TEST_F(HeaderSearchTest, BackSlashWithDotDot) {
+  addSearchDir("..\\y");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+   /*WorkingDir=*/"C:/x/y/"),
+"z/t");
+}
 #endif
 
 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1715,6 +1715,11 @@
 break;
   }
 
+  // Ignore path separators.
+  if (NI->size() == 1 && DI->size() == 1 &&
+  path::is_separator(NI->front()) && path::is_separator(DI->front()))
+continue;
+
   if (*NI != *DI)
 break;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

2019-06-06 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 203386.
gchatelet added a comment.

- Add documentation.
- Fix permissive HasNoRuntimeAttribute
- Mark interleave as disabled in the documentation.
- Use no-builtin instead of no-runtime-for.
- Adding an llvm.memcpy.inline intrinsic.
- Adding __builtin_memcpy_inline clang builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
  llvm/test/CodeGen/X86/memcpy-inline.ll
  llvm/test/CodeGen/X86/memcpy.ll

Index: llvm/test/CodeGen/X86/memcpy.ll
===
--- llvm/test/CodeGen/X86/memcpy.ll
+++ llvm/test/CodeGen/X86/memcpy.ll
@@ -7,7 +7,7 @@
 
 
 ; Variable memcpy's should lower to calls.
-define i8* @test1(i8* %a, i8* %b, i64 %n) nounwind {
+define void @test1(i8* %a, i8* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test1:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -17,11 +17,11 @@
 ; DARWIN-NEXT:jmp _memcpy ## TAILCALL
 entry:
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 )
-	ret i8* %a
+  ret void
 }
 
 ; Variable memcpy's should lower to calls.
-define i8* @test2(i64* %a, i64* %b, i64 %n) nounwind {
+define void @test2(i64* %a, i64* %b, i64 %n) nounwind {
 ; LINUX-LABEL: test2:
 ; LINUX:   # %bb.0: # %entry
 ; LINUX-NEXT:jmp memcpy # TAILCALL
@@ -33,7 +33,25 @@
 	%tmp14 = bitcast i64* %a to i8*
 	%tmp25 = bitcast i64* %b to i8*
 	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %tmp14, i8* align 8 %tmp25, i64 %n, i1 0 )
-	ret i8* %tmp14
+  ret void
+}
+
+; Variable length memcpy's with disabled runtime should lower to repmovsb.
+define void @memcpy_no_runtime(i8* %a, i8* %b, i64 %n) nounwind {
+; LINUX-LABEL: memcpy_no_runtime:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:movq %rdx, %rcx
+; LINUX-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; LINUX-NEXT:retq
+;
+; DARWIN-LABEL: memcpy_no_runtime:
+; DARWIN:   ## %bb.0: ## %entry
+; DARWIN-NEXT:movq %rdx, %rcx
+; DARWIN-NEXT:rep;movsb (%rsi), %es:(%rdi)
+; DARWIN-NEXT:retq
+entry:
+	tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i1 0 ) "no-builtin-memcpy"
+  ret void
 }
 
 ; Large constant memcpy's should lower to a call when optimizing for size.
Index: llvm/test/CodeGen/X86/memcpy-inline.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/memcpy-inline.ll
@@ -0,0 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 | FileCheck %s
+
+declare void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1) nounwind
+
+define void @test1(i8* %a, i8* %b) nounwind {
+; CHECK-LABEL: test1:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:movq (%rsi), %rax
+; CHECK-NEXT:movq %rax, (%rdi)
+; CHECK-NEXT:retq
+	tail call void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* %a, i8* %b, i64 8, i1 0 )
+  ret void
+}
Index: llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
===
--- llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -314,5 +314,9 @@
   Size.getValueType(), Align, isVolatile,
   AlwaysInline, DstPtrInfo, SrcPtrInfo);
 
+  /// Handle runtime sizes through repmovsb when we AlwaysInline.
+  if (AlwaysInline)
+return emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src, Size, MVT::i8);
+
   return SDValue();
 }
Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -96,6 +96,14 @@
   return II;
 }
 
+static void ForwardAttribute(const Function *F, StringRef Attribute,
+ CallInst *CI) {
+  if (F->hasFnAttribute(Attribute)) {
+CI->addAttribute(AttributeList::FunctionIndex,
+ F->getFnAttribute(Attribute));
+  }
+}
+
 CallInst *IRBuilderBase::
 CreateMemSet(Value *Ptr, Value *Val, Value *Size, unsigned Align,
  bool isVolatile, MDNode *TBAATag, MDNode *ScopeTag,
@@ -103,7 +111,8 @@
   Ptr = getCastedInt8PtrValue(Ptr);
   Value *Ops[] = {Ptr, Val, Size, getInt1(isVolatile)};
   Type *Tys[] = { Ptr->getType(), Size->getType() };
-  Module *M = 

[PATCH] D46308: [PATCH 2/3] [RFC only] clang implementation of sizeless types

2019-06-06 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm abandoned this revision.
rsandifo-arm added a comment.
Herald added a subscriber: jfb.
Herald added a reviewer: rengolin.
Herald added a project: clang.

Abandoning in favour of:

https://reviews.llvm.org/D62960
https://reviews.llvm.org/D62961
https://reviews.llvm.org/D62962


Repository:
  rC Clang

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

https://reviews.llvm.org/D46308



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


[PATCH] D46307: [PATCH 1/3] [RFC only] Hack to add some sizeless built-in types

2019-06-06 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm abandoned this revision.
rsandifo-arm added a comment.
Herald added a subscriber: arphaman.
Herald added a reviewer: rengolin.
Herald added a project: clang.

Abandoning in favour of:

https://reviews.llvm.org/D62960
https://reviews.llvm.org/D62961
https://reviews.llvm.org/D62962


Repository:
  rC Clang

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

https://reviews.llvm.org/D46307



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


[PATCH] D62961: [AST] Add new Type queries for sizeless types

2019-06-06 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

This patch adds new type queries:

- isSizelessType
- isDefiniteType
- isIndefiniteType

following the type classification described in the comments.
The follow-on patch to support sizeless types in C and C++
has more details about the sizeless type extension.


Repository:
  rC Clang

https://reviews.llvm.org/D62961

Files:
  include/clang/AST/CanonicalType.h
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  unittests/AST/CMakeLists.txt
  unittests/AST/SizelessTypesTest.cpp

Index: unittests/AST/SizelessTypesTest.cpp
===
--- /dev/null
+++ unittests/AST/SizelessTypesTest.cpp
@@ -0,0 +1,105 @@
+//===- unittests/AST/SizelessTypesTest.cpp --- Sizeless type tests ===//
+//
+// 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
+//
+//===--===//
+//
+// This file contains tests for clang::Type queries related to sizeless types.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+
+struct SizelessTypeTester : public ::testing::Test {
+  // Declare an incomplete structure type.
+  std::unique_ptr AST = tooling::buildASTFromCode("struct foo;");
+  ASTContext  = AST->getASTContext();
+  TranslationUnitDecl  = *Ctx.getTranslationUnitDecl();
+  TypeDecl *Foo = cast(TU.lookup(("foo")).front());
+  const Type *FooTy = Foo->getTypeForDecl();
+};
+
+TEST_F(SizelessTypeTester, TestSizeless) {
+  ASSERT_TRUE(Ctx.SveInt8Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveInt16Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveInt32Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveInt64Ty->isSizelessType());
+
+  ASSERT_TRUE(Ctx.SveUint8Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveUint16Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveUint32Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveUint64Ty->isSizelessType());
+
+  ASSERT_TRUE(Ctx.SveFloat16Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveFloat32Ty->isSizelessType());
+  ASSERT_TRUE(Ctx.SveFloat64Ty->isSizelessType());
+
+  ASSERT_TRUE(Ctx.SveBoolTy->isSizelessType());
+
+  ASSERT_FALSE(Ctx.VoidTy->isSizelessType());
+  ASSERT_FALSE(Ctx.PseudoObjectTy->isSizelessType());
+  ASSERT_FALSE(FooTy->isSizelessType());
+
+  ASSERT_FALSE(Ctx.getPointerType(Ctx.SveBoolTy)->isSizelessType());
+  ASSERT_FALSE(Ctx.getLValueReferenceType(Ctx.SveBoolTy)->isSizelessType());
+  ASSERT_FALSE(Ctx.getRValueReferenceType(Ctx.SveBoolTy)->isSizelessType());
+}
+
+TEST_F(SizelessTypeTester, TestIndefinite) {
+  ASSERT_FALSE(Ctx.SveInt8Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveInt16Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveInt32Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveInt64Ty->isIndefiniteType());
+
+  ASSERT_FALSE(Ctx.SveUint8Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveUint16Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveUint32Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveUint64Ty->isIndefiniteType());
+
+  ASSERT_FALSE(Ctx.SveFloat16Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveFloat32Ty->isIndefiniteType());
+  ASSERT_FALSE(Ctx.SveFloat64Ty->isIndefiniteType());
+
+  ASSERT_FALSE(Ctx.SveBoolTy->isIndefiniteType());
+
+  ASSERT_TRUE(Ctx.VoidTy->isIndefiniteType());
+  ASSERT_FALSE(Ctx.PseudoObjectTy->isIndefiniteType());
+  ASSERT_TRUE(FooTy->isIndefiniteType());
+
+  ASSERT_FALSE(Ctx.getPointerType(Ctx.SveBoolTy)->isIndefiniteType());
+  ASSERT_FALSE(Ctx.getLValueReferenceType(Ctx.SveBoolTy)->isIndefiniteType());
+  ASSERT_FALSE(Ctx.getRValueReferenceType(Ctx.SveBoolTy)->isIndefiniteType());
+}
+
+TEST_F(SizelessTypeTester, TestIncomplete) {
+  ASSERT_TRUE(Ctx.SveInt8Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveInt16Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveInt32Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveInt64Ty->isIncompleteType());
+
+  ASSERT_TRUE(Ctx.SveUint8Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveUint16Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveUint32Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveUint64Ty->isIncompleteType());
+
+  ASSERT_TRUE(Ctx.SveFloat16Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveFloat32Ty->isIncompleteType());
+  ASSERT_TRUE(Ctx.SveFloat64Ty->isIncompleteType());
+
+  ASSERT_TRUE(Ctx.SveBoolTy->isIncompleteType());
+
+  ASSERT_TRUE(Ctx.VoidTy->isIncompleteType());
+  ASSERT_FALSE(Ctx.PseudoObjectTy->isIncompleteType());
+  ASSERT_TRUE(FooTy->isIncompleteType());
+
+  ASSERT_FALSE(Ctx.getPointerType(Ctx.SveBoolTy)->isIncompleteType());
+  ASSERT_FALSE(Ctx.getLValueReferenceType(Ctx.SveBoolTy)->isIncompleteType());
+  

[PATCH] D62962: Clang implementation of sizeless types

2019-06-06 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm created this revision.
Herald added subscribers: cfe-commits, jfb, kristof.beyls, javed.absar.
Herald added a reviewer: rengolin.
Herald added a project: clang.

This patch adds the concept of "sizeless" types to C and C++.
The documentation part of the patch describes the extension in
detail and explains the rationale.  The patch is a prerequisite
for adding AArch64 SVE intrinsic functions.

The main change is to make several checks use the new distinction
between "definite" and "indefinite" types instead of the usual
distinction between "complete" and "incomplete" types.  There are
also a couple of places that check specifically for sizeless types.

The patch doesn't reword the diagnostics to talk about indefinite
types rather than incomplete types since (a) "indefinite" won't
mean much to most users and (b) the patch shouldn't affect the user
experience for non-AArch64 targets.

The FIXMEs about inline asms are resolved by later SVE patches.

The change to DiagnoseForRangeVariableCopies is for consistency.
As the tests show, DiagnoseForRangeReferenceVariableCopies can apply
to sizeless types, but at the moment DiagnoseForRangeConstVariableCopies
(which is the only part affected by the completeness check) doesn't
warn for PODs, and so doesn't warn for sizeless types either.


Repository:
  rC Clang

https://reviews.llvm.org/D62962

Files:
  docs/SizelessTypes.rst
  include/clang/AST/Expr.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Initialization.h
  include/clang/Sema/Sema.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaFixItUtils.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaPseudoObject.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/SemaType.cpp
  test/Sema/sizeless-1.c
  test/SemaCXX/sizeless-1.cpp

Index: test/SemaCXX/sizeless-1.cpp
===
--- /dev/null
+++ test/SemaCXX/sizeless-1.cpp
@@ -0,0 +1,533 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++11 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++14 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++14 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+
+namespace std { struct type_info; }
+
+typedef __SVInt8_t svint8_t;
+typedef __SVInt16_t svint16_t;
+
+svint8_t global_int8; // expected-error {{non-local variable with sizeless type}}
+extern svint8_t extern_int8; // expected-error {{non-local variable with sizeless type}}
+static svint8_t static_int8; // expected-error {{non-local variable with sizeless type}}
+svint8_t *global_int8_ptr;
+extern svint8_t *extern_int8_ptr;
+static svint8_t *static_int8_ptr;
+
+typedef svint8_t int8_typedef;
+typedef svint8_t *int8_ptr_typedef;
+
+int sizeof_int8 = sizeof(svint8_t); // expected-error {{incomplete type}}
+
+#if __cplusplus >= 201103L
+int alignof_int8 = alignof(svint8_t); // expected-error {{incomplete type}}
+#else
+int alignof_int8 = _Alignof(svint8_t); // expected-error {{incomplete type}}
+#endif
+
+void pass_int8(svint8_t); // expected-note {{no known conversion}}
+
+extern "C" svint8_t return_int8();
+
+typedef svint8_t vec_int8 __attribute__((vector_size(64))); // expected-error {{invalid vector element type}}
+
+void dump(const volatile void *);
+
+void overf(svint8_t);
+void overf(svint16_t);
+
+void overf8(svint8_t); // expected-note + {{not viable}}
+void overf8(int); // expected-note + {{not viable}}
+
+void overf16(svint16_t); // expected-note + {{not viable}}
+void overf16(int); // expected-note + {{not viable}}
+
+void varargs(int, ...);
+
+void unused() {
+  svint8_t unused_int8; // expected-warning {{unused}}
+}
+
+void func(int sel) {
+  svint8_t local_int8;
+  svint16_t local_int16;
+
+  svint8_t __attribute__((aligned)) aligned_int8_1; // 

[PATCH] D62960: SVE opaque type for C intrinsics demo

2019-06-06 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm created this revision.
Herald added subscribers: cfe-commits, arphaman, tschuett, javed.absar.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added a project: clang.

A lightly-modified version of Graham Hunter's original patch:
https://reviews.llvm.org/D59245

The patch adds types that are used by the sizeless type RFC.


Repository:
  rC Clang

https://reviews.llvm.org/D62960

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/AArch64SVEACLETypes.def
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/PrintfFormatString.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1527,6 +1527,12 @@
   case BuiltinType::OCLClkEvent:
   case BuiltinType::OCLQueue:
   case BuiltinType::OCLReserveID:
+  // SVE Types
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+  case BuiltinType::Id:
+#define SVE_PREDICATE_TYPE(Name, Id, SingletonId, ElKind)\
+  case BuiltinType::Id:
+#include "clang/Basic/AArch64SVEACLETypes.def"
 #define BUILTIN_TYPE(Id, SingletonId)
 #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
 #define UNSIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -7037,6 +7037,16 @@
 case PREDEF_TYPE_OMP_ARRAY_SECTION:
   T = Context.OMPArraySectionTy;
   break;
+// SVE Types
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+case PREDEF_TYPE_##Id##_ID: \
+  T = Context.SingletonId; \
+  break;
+#define SVE_PREDICATE_TYPE(Name, Id, SingletonId, ElKind)\
+case PREDEF_TYPE_##Id##_ID: \
+  T = Context.SingletonId; \
+  break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
 }
 
 assert(!T.isNull() && "Unknown predefined type");
Index: lib/Serialization/ASTCommon.cpp
===
--- lib/Serialization/ASTCommon.cpp
+++ lib/Serialization/ASTCommon.cpp
@@ -232,6 +232,16 @@
   case BuiltinType::OCLReserveID:
 ID = PREDEF_TYPE_RESERVE_ID_ID;
 break;
+// SVE Types
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+  case BuiltinType::Id: \
+ID = PREDEF_TYPE_##Id##_ID; \
+break;
+#define SVE_PREDICATE_TYPE(Name, Id, SingletonId, ElKind) \
+  case BuiltinType::Id: \
+ID = PREDEF_TYPE_##Id##_ID; \
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   case BuiltinType::BuiltinFn:
 ID = PREDEF_TYPE_BUILTIN_FN;
 break;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5243,6 +5243,12 @@
 #define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
   case BuiltinType::Id:
 #include "clang/Basic/OpenCLExtensionTypes.def"
+// SVE Types
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+  case BuiltinType::Id:
+#define SVE_PREDICATE_TYPE(Name, Id, SingletonId, ElKind) \
+  case BuiltinType::Id:
+#include "clang/Basic/AArch64SVEACLETypes.def"
 #define PLACEHOLDER_TYPE(ID, SINGLETON_ID)
 #define BUILTIN_TYPE(ID, SINGLETON_ID) case BuiltinType::ID:
 #include "clang/AST/BuiltinTypes.def"
@@ -17049,6 +17055,12 @@
 #define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
   case BuiltinType::Id:
 #include "clang/Basic/OpenCLExtensionTypes.def"
+// SVE Types
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+  case BuiltinType::Id:
+#define SVE_PREDICATE_TYPE(Name, Id, SingletonId, ElKind) \
+  case BuiltinType::Id:
+#include "clang/Basic/AArch64SVEACLETypes.def"
 #define BUILTIN_TYPE(Id, SingletonId) case BuiltinType::Id:
 #define PLACEHOLDER_TYPE(Id, SingletonId)
 #include "clang/AST/BuiltinTypes.def"
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -336,7 +336,15 @@
 addImplicitTypedef(#ExtType, Context.Id##Ty); \
 setOpenCLExtensionForType(Context.Id##Ty, #Ext);
 #include "clang/Basic/OpenCLExtensionTypes.def"
-};
+  };
+
+  // SVE Types
+  // TODO: Check target?
+#define SVE_VECTOR_TYPE(Name, Id, SingletonId, ElKind, ElBits, IsSigned, IsFP)\
+  addImplicitTypedef(Name, Context.SingletonId);
+#define 

[PATCH] D62958: [clang-tidy] Fix descriptions for modernize-make-unique/shared checks.

2019-06-06 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: gribozavr.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62958

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp

Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -80,6 +80,7 @@
 std::unique_ptr getPointer() {
   return std::unique_ptr(new Base);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: note: change to std::make_unique
   // CHECK-FIXES: return std::make_unique();
 }
 
@@ -153,6 +154,7 @@
   // OK to replace for reset and assign
   Pderived.reset(new Derived());
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_unique instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: note: change to std::make_unique
   // CHECK-FIXES: Pderived = std::make_unique();
 
   Pderived = std::unique_ptr(new Derived());
@@ -301,6 +303,7 @@
   // Initialization with the initializer-list constructor.
   std::unique_ptr PE2 = std::unique_ptr(new E{1, 2});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-MESSAGES-NOT: :[[@LINE-2]]:28: note: change to std::make_unique
   // CHECK-FIXES: std::unique_ptr PE2 = std::unique_ptr(new E{1, 2});
   PE2.reset(new E{1, 2});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
Index: clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
@@ -35,6 +35,7 @@
 std::shared_ptr getPointer() {
   return std::shared_ptr(new Base);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_shared instead
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: note: change to std::make_shared
   // CHECK-FIXES: return std::make_shared();
 }
 
@@ -45,6 +46,7 @@
 
   P1.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared]
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: change to std::make_shared
   // CHECK-FIXES: P1 = std::make_shared();
 
   P1 = std::shared_ptr(new int());
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -59,8 +59,8 @@
   const CXXMemberCallExpr *Member, const CXXNewExpr *New);
 
   /// Returns true when the fixes for replacing CXXNewExpr are generated.
-  bool replaceNew(DiagnosticBuilder , const CXXNewExpr *New,
-  SourceManager , ASTContext *Ctx);
+  bool replaceNew(const CXXNewExpr *New, SourceManager , ASTContext *Ctx,
+  std::vector );
   void insertHeader(DiagnosticBuilder , FileID FD);
 };
 
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -159,17 +159,22 @@
   if (Invalid)
 return;
 
-  auto Diag = diag(ConstructCallStart, "use %0 instead")
-  << MakeSmartPtrFunctionName;
+  diag(ConstructCallStart, "use %0 instead") << MakeSmartPtrFunctionName;
 
   // Disable the fix in macros.
   if (InMacro) {
 return;
   }
 
-  if (!replaceNew(Diag, New, SM, Ctx)) {
+  std::vector Fixes;
+  if (!replaceNew(New, SM, Ctx, Fixes)) {
 return;
   }
+  auto FixDiag = diag(ConstructCallStart, "change to %0", DiagnosticIDs::Note)
+ << MakeSmartPtrFunctionName;
+
+  for (const auto  : Fixes)
+FixDiag << Fix;
 
   // Find the location of the template's left angle.
   size_t LAngle = ExprStr.find("<");
@@ -178,14 +183,13 @@
 // If the template argument is missing (because it is part of the alias)
 // we have to add it back.
 ConstructCallEnd = ConstructCallStart.getLocWithOffset(ExprStr.size());
-Diag << FixItHint::CreateInsertion(
-ConstructCallEnd,
-"<" + GetNewExprName(New, SM, getLangOpts()) + ">");
+FixDiag << FixItHint::CreateInsertion(
+ConstructCallEnd, "<" + GetNewExprName(New, SM, getLangOpts()) + ">");
   } else {
 ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle);
   }
 
-  Diag << FixItHint::CreateReplacement(
+  FixDiag << FixItHint::CreateReplacement(
   

[PATCH] D57795: [RISCV] Add FreeBSD targets

2019-06-06 Thread James Clarke via Phabricator via cfe-commits
jrtc27 updated this revision to Diff 203359.
jrtc27 added a comment.

Sorted OS case statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57795

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/freebsd.c


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -376,15 +376,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV32TargetInfo(Triple, Opts);
+default:
+  return new RISCV32TargetInfo(Triple, Opts);
+}
+
   case llvm::Triple::riscv64:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+switch (os) {
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
+case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV64TargetInfo(Triple, Opts);
+default:
+  return new RISCV64TargetInfo(Triple, Opts);
+}
 
   case llvm::Triple::sparc:
 switch (os) {


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -63,6 +63,15 @@
 // RUN:   | FileCheck --check-prefix=CHECK-MIPSN32EL-LD %s
 // CHECK-MIPSN32EL-LD: ld{{.*}}" {{.*}} "-m" "elf32ltsmipn32_fbsd"
 //
+// Check that RISC-V passes the correct linker emulation.
+//
+// RUN: %clang -target riscv32-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32I-LD %s
+// CHECK-RV32I-LD: ld{{.*}}" {{.*}} "-m" "elf32lriscv"
+// RUN: %clang -target riscv64-freebsd %s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64I-LD %s
+// CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
+//
 // Check that the new linker flags are passed to FreeBSD
 // RUN: %clang -no-canonical-prefixes -target x86_64-pc-freebsd8 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -197,6 +197,14 @@
 else
   CmdArgs.push_back("elf64ltsmip_fbsd");
 break;
+  case llvm::Triple::riscv32:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32lriscv");
+break;
+  case llvm::Triple::riscv64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64lriscv");
+break;
   default:
 break;
   }
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -376,15 +376,26 @@
 return new AMDGPUTargetInfo(Triple, Opts);
 
   case llvm::Triple::riscv32:
-// TODO: add cases for FreeBSD, NetBSD, RTEMS once tested.
-if (os == llvm::Triple::Linux)
+// TODO: add cases for NetBSD, RTEMS once tested.
+

[PATCH] D37813: clang-format: better handle namespace macros

2019-06-06 Thread Manuel Klimek via Phabricator via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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


[PATCH] D61681: [clangd] A code tweak to expand a macro

2019-06-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 203358.
ilya-biryukov added a comment.

- Move logically separate parts to other changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61681

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang/include/clang/Tooling/Syntax/Tokens.h

Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -66,6 +66,15 @@
 
   unsigned length() const { return End - Begin; }
 
+  /// Check if \p Offset is inside the range.
+  bool contains(unsigned Offset) const {
+return Begin <= Offset && Offset < End;
+  }
+  /// Check \p Offset is inside the range or equal to its endpoint.
+  bool touches(unsigned Offset) const {
+return Begin <= Offset && Offset <= End;
+  }
+
   /// Gets the substring that this FileRange refers to.
   llvm::StringRef text(const SourceManager ) const;
 
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -216,6 +216,99 @@
   checkTransform(ID, Input, Output);
 }
 
+TEST(TweakTest, ExpandMacro) {
+  llvm::StringLiteral ID = "ExpandMacro";
+
+  // Available on macro names, not available anywhere else.
+  checkAvailable(ID, R"cpp(
+#define FOO 1 2 3
+#define FUNC(X) X+X+X
+^F^O^O^ BAR ^F^O^O^
+^F^U^N^C^(1)
+)cpp");
+  checkNotAvailable(ID, R"cpp(
+^#^d^efine^ ^FO^O 1 ^2 ^3^
+FOO ^B^A^R^ FOO ^
+FUNC(^1^)^
+)cpp");
+
+  // Works as expected on object-like macros.
+  checkTransform(ID, R"cpp(
+#define FOO 1 2 3
+^FOO BAR FOO
+)cpp",
+ R"cpp(
+#define FOO 1 2 3
+1 2 3 BAR FOO
+)cpp");
+  checkTransform(ID, R"cpp(
+#define FOO 1 2 3
+FOO BAR ^FOO
+)cpp",
+ R"cpp(
+#define FOO 1 2 3
+FOO BAR 1 2 3
+)cpp");
+
+  // And function-like macros.
+  checkTransform(ID, R"cpp(
+#define FUNC(X) X+X+X
+F^UNC(2)
+)cpp",
+ R"cpp(
+#define FUNC(X) X+X+X
+2 + 2 + 2
+)cpp");
+
+  // Works on empty macros.
+  checkTransform(ID, R"cpp(
+#define EMPTY
+int a ^EMPTY;
+  )cpp",
+ R"cpp(
+#define EMPTY
+int a ;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY_FN(X)
+int a ^EMPTY_FN(1 2 3);
+  )cpp",
+ R"cpp(
+#define EMPTY_FN(X)
+int a ;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 ^EMPTY EMPTY_FN(1);
+  )cpp",
+ R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123  EMPTY_FN(1);
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 ^EMPTY_FN(1) EMPTY;
+  )cpp",
+ R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123  EMPTY;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 EMPTY_FN(1) ^EMPTY;
+  )cpp",
+ R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 EMPTY_FN(1) ;
+  )cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
@@ -0,0 +1,125 @@
+//===--- ExpandMacro.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 "refactor/Tweak.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include 
+namespace clang {
+namespace clangd {
+namespace {
+/// Replaces a reference to a macro under cursor to its expansion.
+/// Before:
+///   #define FOO(X) X+X
+///   FOO(10*a)
+///   ^^^
+/// After:
+///   #define FOO(X) X+X
+///   10*a+10*a
+class ExpandMacro : public Tweak {
+public:
+  const char *id() const override final;
+
+  bool prepare(const Selection ) override;
+  Expected apply(const Selection ) override;
+  std::string title() const override;
+
+private:
+  syntax::TokenBuffer::Expansion Expansion;
+};
+
+REGISTER_TWEAK(ExpandMacro)
+
+/// Finds a spelled token that the cursor is pointing at.
+static const syntax::Token *
+findTokenUnderCursor(const SourceManager ,
+ 

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

2019-06-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: aaron.ballman.
Szelethus added a comment.

Added Aaron as he wrote the this diagnostic output type :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952



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


[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

2019-06-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
Herald added a project: clang.
ilya-biryukov added a child revision: D61681: [clangd] A code tweak to expand a 
macro.

The first use of this is a code tweak to expand macro calls.
Will later be used to build syntax trees.

The memory overhead is small as we only store token of the main file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62956

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt

Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -86,6 +86,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingSyntax
   LLVMSupport
   LLVMTestingSupport
   )
Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -26,5 +26,6 @@
   clangSema
   clangTooling
   clangToolingCore
+  clangToolingSyntax
   ${CLANGD_XPC_LIBS}
   )
Index: clang-tools-extra/clangd/ClangdUnit.h
===
--- clang-tools-extra/clangd/ClangdUnit.h
+++ clang-tools-extra/clangd/ClangdUnit.h
@@ -24,6 +24,7 @@
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include 
 #include 
 #include 
@@ -115,10 +116,14 @@
   const IncludeStructure () const;
   const CanonicalIncludes () const;
 
+  /// Tokens recorded while parsing the main file.
+  /// (!) does not have tokens from the preamble.
+  const syntax::TokenBuffer () const { return Tokens; }
+
 private:
   ParsedAST(std::shared_ptr Preamble,
 std::unique_ptr Clang,
-std::unique_ptr Action,
+std::unique_ptr Action, syntax::TokenBuffer Tokens,
 std::vector LocalTopLevelDecls, std::vector Diags,
 IncludeStructure Includes, CanonicalIncludes CanonIncludes);
 
@@ -132,6 +137,9 @@
   // FrontendAction.EndSourceFile).
   std::unique_ptr Clang;
   std::unique_ptr Action;
+  /// Expanded tokens for the main file. Does not contain tokens for the file
+  /// preamble.
+  syntax::TokenBuffer Tokens;
 
   // Data, stored after parsing.
   std::vector Diags;
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -36,6 +36,7 @@
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -418,6 +419,9 @@
   collectIWYUHeaderMaps();
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
 
+  // Collect tokens of the main file.
+  syntax::TokenCollector Tokens(Clang->getPreprocessor());
+
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
@@ -446,8 +450,9 @@
   if (Preamble)
 Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-   std::move(ParsedDecls), std::move(Diags),
-   std::move(Includes), std::move(CanonIncludes));
+   std::move(Tokens).consume(), std::move(ParsedDecls),
+   std::move(Diags), std::move(Includes),
+   std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &) = default;
@@ -540,11 +545,13 @@
 ParsedAST::ParsedAST(std::shared_ptr Preamble,
  std::unique_ptr Clang,
  std::unique_ptr Action,
+ syntax::TokenBuffer Tokens,
  std::vector LocalTopLevelDecls,
  std::vector Diags, IncludeStructure Includes,
  CanonicalIncludes CanonIncludes)
 : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
-  Action(std::move(Action)), Diags(std::move(Diags)),
+  Action(std::move(Action)), Tokens(std::move(Tokens)),
+  Diags(std::move(Diags)),
   LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
   Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
   assert(this->Clang);
Index: clang-tools-extra/clangd/CMakeLists.txt

[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

2019-06-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: kadircet.
Herald added a project: clang.

Used in clangd for a code tweak that expands a macro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62954

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp


Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -200,6 +200,32 @@
   : LastSpelled + 1);
 }
 
+llvm::Optional
+TokenBuffer::findExpansion(const syntax::Token *Spelled) const {
+  assert(Spelled);
+  assert(Spelled->location().isFileID() && "not a spelled token");
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end());
+
+  auto  = FileIt->second;
+  assert(File.SpelledTokens.data() <= Spelled &&
+ Spelled < (File.SpelledTokens.data() + File.SpelledTokens.size()));
+
+  unsigned SpelledIndex = Spelled - File.SpelledTokens.data();
+  auto M = llvm::bsearch(File.Mappings, [&](const Mapping ) {
+return SpelledIndex <= M.BeginSpelled;
+  });
+  if (M == File.Mappings.end() || M->BeginSpelled != SpelledIndex)
+return llvm::None;
+
+  Expansion E;
+  E.Spelled = llvm::makeArrayRef(File.SpelledTokens.data() + M->BeginSpelled,
+ File.SpelledTokens.data() + M->EndSpelled);
+  E.Expanded = llvm::makeArrayRef(ExpandedTokens.data() + M->BeginExpanded,
+  ExpandedTokens.data() + M->EndExpanded);
+  return E;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager 
,
 const LangOptions ) {
   std::vector Tokens;
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -200,6 +200,21 @@
   llvm::Optional>
   spelledForExpanded(llvm::ArrayRef Expanded) const;
 
+  /// An expansion produced by the preprocessor, includes macro expansions and
+  /// macro directives. Preprocessor always maps a non-empty range of spelled
+  /// tokens to a (possibly empty) range of expanded tokens.
+  /// Here is a few examples of expansions:
+  ///#pragme once  // Expansion from "#pragma once" to an empty range.
+  ///#define FOO 1 2 3 // Expansion from "#define FOO 1" to an empty range.
+  ///FOO   // Expansion from "FOO" to "1 2 3".
+  struct Expansion {
+llvm::ArrayRef Spelled;
+llvm::ArrayRef Expanded;
+  };
+  /// If \p Spelled starts a mapping (e.g. if it's a macro name) return the
+  /// subrange of expanded tokens that the macro expands to.
+  llvm::Optional findExpansion(const syntax::Token *Spelled) const;
+
   /// Lexed tokens of a file before preprocessing. E.g. for the following input
   /// #define DECL(name) int name = 10
   /// DECL(a);
@@ -292,6 +307,7 @@
   /// finished, i.e. after running Execute().
   LLVM_NODISCARD TokenBuffer consume() &&;
 
+
 private:
   /// Maps from a start location to an end location of each mapping.
   ///   1. range from '#' to the last token in the line for PP directives,


Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -200,6 +200,32 @@
   : LastSpelled + 1);
 }
 
+llvm::Optional
+TokenBuffer::findExpansion(const syntax::Token *Spelled) const {
+  assert(Spelled);
+  assert(Spelled->location().isFileID() && "not a spelled token");
+  auto FileIt = Files.find(SourceMgr->getFileID(Spelled->location()));
+  assert(FileIt != Files.end());
+
+  auto  = FileIt->second;
+  assert(File.SpelledTokens.data() <= Spelled &&
+ Spelled < (File.SpelledTokens.data() + File.SpelledTokens.size()));
+
+  unsigned SpelledIndex = Spelled - File.SpelledTokens.data();
+  auto M = llvm::bsearch(File.Mappings, [&](const Mapping ) {
+return SpelledIndex <= M.BeginSpelled;
+  });
+  if (M == File.Mappings.end() || M->BeginSpelled != SpelledIndex)
+return llvm::None;
+
+  Expansion E;
+  E.Spelled = llvm::makeArrayRef(File.SpelledTokens.data() + M->BeginSpelled,
+ File.SpelledTokens.data() + M->EndSpelled);
+  E.Expanded = llvm::makeArrayRef(ExpandedTokens.data() + M->BeginExpanded,
+  ExpandedTokens.data() + M->EndExpanded);
+  return E;
+}
+
 std::vector syntax::tokenize(FileID FID, const SourceManager ,
 const LangOptions ) {
   std::vector Tokens;
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ 

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a project: clang.

This change makes sure we have a single mapping for each macro expansion,
even if the result of expansion was empty.

To achieve that, we take information from PPCallbacks::MacroExpands into
account. Previously we relied only on source locations of expanded tokens.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62953

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -420,7 +420,9 @@
   spelled tokens:
 # define EMPTY # define EMPTY_FUNC ( X ) EMPTY EMPTY_FUNC ( 1 + 2 + 3 )
   mappings:
-['#'_0, ''_18) => [''_0, ''_0)
+['#'_0, 'EMPTY'_9) => [''_0, ''_0)
+['EMPTY'_9, 'EMPTY_FUNC'_10) => [''_0, ''_0)
+['EMPTY_FUNC'_10, ''_18) => [''_0, ''_0)
 )"},
   // File ends with a macro replacement.
   {R"cpp(
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -14,6 +14,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -226,6 +227,33 @@
   return Tokens;
 }
 
+class TokenCollector::Callbacks : public PPCallbacks {
+public:
+  Callbacks(TokenCollector ) : C() {}
+
+  /// Disabled instance will stop reporting anything to TokenCollector.
+  void disable() { C = nullptr; }
+
+  void MacroExpands(const clang::Token , const MacroDefinition ,
+SourceRange Range, const MacroArgs *Args) override {
+if (!C)
+  return;
+// Do not record recursive expansions.
+if (!MacroNameTok.getLocation().isFileID() ||
+(LastExpansionEnd.isValid() &&
+ C->PP.getSourceManager().isBeforeInTranslationUnit(Range.getBegin(),
+LastExpansionEnd)))
+  return;
+C->Mappings[Range.getBegin().getRawEncoding()] = Range.getEnd();
+LastExpansionEnd = Range.getEnd();
+  }
+  // FIXME: handle #pragma, #include, etc.
+private:
+  TokenCollector *C;
+  /// Used to detect recursive macro expansions.
+  SourceLocation LastExpansionEnd;
+};
+
 /// Fills in the TokenBuffer by tracing the run of a preprocessor. The
 /// implementation tracks the tokens, macro expansions and directives coming
 /// from the preprocessor and:
@@ -253,15 +281,19 @@
 );
 Expanded.push_back(syntax::Token(T));
   });
+  // And locations of macro calls.
+  auto CB = llvm::make_unique(*this);
+  this->CB = CB.get();
+  PP.addPPCallbacks(std::move(CB));
 }
 
 /// Builds mappings and spelled tokens in the TokenBuffer based on the expanded
 /// token stream.
 class TokenCollector::Builder {
 public:
-  Builder(std::vector Expanded, const SourceManager ,
-  const LangOptions )
-  : Result(SM), SM(SM), LangOpts(LangOpts) {
+  Builder(std::vector Expanded, SpelledMappings Mappings,
+  const SourceManager , const LangOptions )
+  : Result(SM), Mappings(std::move(Mappings)), SM(SM), LangOpts(LangOpts) {
 Result.ExpandedTokens = std::move(Expanded);
   }
 
@@ -330,22 +362,17 @@
 
 fillGapUntil(File, SpelledRange.getBegin(), I);
 
-TokenBuffer::Mapping M;
-// Skip the spelled macro tokens.
-std::tie(M.BeginSpelled, M.EndSpelled) =
-consumeSpelledUntil(File, SpelledRange.getEnd().getLocWithOffset(1));
 // Skip all expanded tokens from the same macro expansion.
-M.BeginExpanded = I;
+unsigned BeginExpanded = I;
 for (; I + 1 < Result.ExpandedTokens.size(); ++I) {
   auto NextL = Result.ExpandedTokens[I + 1].location();
   if (!NextL.isMacroID() ||
   SM.getExpansionLoc(NextL) != SpelledRange.getBegin())
 break;
 }
-M.EndExpanded = I + 1;
-
-// Add a resulting mapping.
-File.Mappings.push_back(M);
+unsigned EndExpanded = I + 1;
+consumeMapping(File, SM.getFileOffset(SpelledRange.getEnd()), BeginExpanded,
+   EndExpanded, NextSpelled[FID]);
   }
 
   /// Initializes TokenBuffer::Files and fills spelled tokens and expanded
@@ -367,69 +394,108 @@
 }
   }
 
-  /// Consumed spelled tokens until location L is reached (token starting at L
-  /// is not included). Returns the indicies of the consumed range.
-  std::pair
-  consumeSpelledUntil(TokenBuffer::MarkedFile , SourceLocation L) {
-assert(L.isFileID());
-FileID FID;
-unsigned Offset;
-std::tie(FID, Offset) = SM.getDecomposedLoc(L);
+  void 

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

2019-06-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The `%diff_sarif` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies a change similar to the one described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically. 
`%diff_sarif` is then, being unused, removed.

The changes were applied via a script, except that the expected files required 
small updates for changes in the length of the test files.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62952

Files:
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
  test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
  test/Analysis/lit.local.cfg


Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,9 +14,12 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
-# Diff command for testing SARIF output to reference output.
-config.substitutions.append(('%diff_sarif',
-'''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
+# Filtering command for testing SARIF output against reference output.
+config.substitutions.append(('%normalize_sarif',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*"uri": "file:.*%basename_t"$',
+ '^[[:space:]]*"version": ".* version .*"$',
+ '^[[:space:]]*"version": "2\.0\.0-csd\.[0-9]*\.beta\.[0-9-]{10}"$')))
 
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
Index: test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o %t && echo >>%t && %normalize_sarif 
<%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif 
>%t.expected.sed.sarif && %normalize_sarif <%t | diff -U1 -b 
%t.expected.sed.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
===
--- test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
+++ test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o %t && echo >>%t && %normalize_sarif 
<%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif 
>%t.expected.sed.sarif && %normalize_sarif <%t | diff -U1 -b 
%t.expected.sed.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -7,7 +7,7 @@
   "fileLocation": {
 "uri": "file:sarif-multi-diagnostic-test.c"
   },
-  "length": 667,
+  "length": 771,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ 

[PATCH] D62621: [LibTooling] Add insert/remove convenience functions for creating `ASTEdit`s.

2019-06-06 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362707: [LibTooling] Add insert/remove convenience functions 
for creating `ASTEdit`s. (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62621?vs=203122=203351#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62621

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
  cfe/trunk/unittests/Tooling/TransformerTest.cpp


Index: cfe/trunk/unittests/Tooling/TransformerTest.cpp
===
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp
@@ -349,6 +349,64 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, InsertBeforeEdit) {
+  std::string Input = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Ret = "return";
+  testRule(makeRule(returnStmt().bind(Ret),
+insertBefore(statement(Ret), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, InsertAfterEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int x = 5;
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl),
+insertAfter(statement(Decl), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RemoveEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl), remove(statement(Decl))), Input,
+   Expected);
+}
+
 TEST_F(TransformerTest, MultiChange) {
   std::string Input = R"cc(
 void foo() {
Index: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
@@ -193,6 +193,23 @@
   return change(node(RewriteRule::RootID), std::move(Replacement));
 }
 
+/// Inserts \p Replacement before \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertBefore(RangeSelector S, TextGenerator Replacement) {
+  return change(before(std::move(S)), std::move(Replacement));
+}
+
+/// Inserts \p Replacement after \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertAfter(RangeSelector S, TextGenerator Replacement) {
+  return change(after(std::move(S)), std::move(Replacement));
+}
+
+/// Removes the source selected by \p S.
+inline ASTEdit remove(RangeSelector S) {
+  return change(std::move(S), text(""));
+}
+
 /// The following three functions are a low-level part of the RewriteRule
 /// API. We expose them for use in implementing the fixtures that interpret
 /// RewriteRule, like Transformer and TransfomerTidy, or for more advanced


Index: cfe/trunk/unittests/Tooling/TransformerTest.cpp
===
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp
@@ -349,6 +349,64 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, InsertBeforeEdit) {
+  std::string Input = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Ret = "return";
+  testRule(makeRule(returnStmt().bind(Ret),
+insertBefore(statement(Ret), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, InsertAfterEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int x = 5;
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl),
+insertAfter(statement(Decl), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RemoveEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl), remove(statement(Decl))), Input,
+   Expected);
+}
+
 TEST_F(TransformerTest, MultiChange) {
   std::string Input = R"cc(
 void foo() {
Index: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h

r362707 - [LibTooling] Add insert/remove convenience functions for creating `ASTEdit`s.

2019-06-06 Thread Yitzhak Mandelbaum via cfe-commits
Author: ymandel
Date: Thu Jun  6 07:20:29 2019
New Revision: 362707

URL: http://llvm.org/viewvc/llvm-project?rev=362707=rev
Log:
[LibTooling] Add insert/remove convenience functions for creating `ASTEdit`s.

Summary: `change()` is an all purpose function; the revision adds simple 
shortcuts for the specific operations of inserting (before/after) or removing 
source.

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/unittests/Tooling/TransformerTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=362707=362706=362707=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Thu Jun  6 
07:20:29 2019
@@ -193,6 +193,23 @@ inline ASTEdit change(TextGenerator Repl
   return change(node(RewriteRule::RootID), std::move(Replacement));
 }
 
+/// Inserts \p Replacement before \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertBefore(RangeSelector S, TextGenerator Replacement) {
+  return change(before(std::move(S)), std::move(Replacement));
+}
+
+/// Inserts \p Replacement after \p S, leaving the source selected by \S
+/// unchanged.
+inline ASTEdit insertAfter(RangeSelector S, TextGenerator Replacement) {
+  return change(after(std::move(S)), std::move(Replacement));
+}
+
+/// Removes the source selected by \p S.
+inline ASTEdit remove(RangeSelector S) {
+  return change(std::move(S), text(""));
+}
+
 /// The following three functions are a low-level part of the RewriteRule
 /// API. We expose them for use in implementing the fixtures that interpret
 /// RewriteRule, like Transformer and TransfomerTidy, or for more advanced

Modified: cfe/trunk/unittests/Tooling/TransformerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/TransformerTest.cpp?rev=362707=362706=362707=diff
==
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp Thu Jun  6 07:20:29 2019
@@ -349,6 +349,64 @@ TEST_F(TransformerTest, NodePartMemberMu
Input, Expected);
 }
 
+TEST_F(TransformerTest, InsertBeforeEdit) {
+  std::string Input = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Ret = "return";
+  testRule(makeRule(returnStmt().bind(Ret),
+insertBefore(statement(Ret), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, InsertAfterEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  int x = 5;
+  int y = 3;
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl),
+insertAfter(statement(Decl), text("int y = 3;"))),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RemoveEdit) {
+  std::string Input = R"cc(
+int f() {
+  int x = 5;
+  return 7;
+}
+  )cc";
+  std::string Expected = R"cc(
+int f() {
+  return 7;
+}
+  )cc";
+
+  StringRef Decl = "decl";
+  testRule(makeRule(declStmt().bind(Decl), remove(statement(Decl))), Input,
+   Expected);
+}
+
 TEST_F(TransformerTest, MultiChange) {
   std::string Input = R"cc(
 void foo() {


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


[PATCH] D62951: [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

2019-06-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
hubert.reinterpretcast added parent revisions: D62949: [analyzer][tests] Add 
normalize_plist to replace diff_plist, D62950: [analyzer][tests] Use 
normalize_plist in place of diff_plist (`cat` cases).

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically to 
the cases where the output file is piped to `%diff_plist` via `tail`. 
`%diff_plist` is then, being unused, removed.

The changes were applied via a script.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62951

Files:
  test/Analysis/MismatchedDeallocator-path-notes.cpp
  test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
  test/Analysis/diagnostics/plist-multi-file.c
  test/Analysis/lambda-notes.cpp
  test/Analysis/lit.local.cfg
  test/Analysis/malloc-plist.c


Index: test/Analysis/malloc-plist.c
===
--- test/Analysis/malloc-plist.c
+++ test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -verify -o %t -analyzer-config eagerly-assume=false %s
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/malloc-plist.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/malloc-plist.c.plist 
>%t.expected.sed && echo >>%t && tail -n +11 %t | %normalize_plist | diff -u 
%t.expected.sed -
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -9,11 +9,6 @@
 config.test_format = analyzer_test.AnalyzerTest(
 config.test_format.execute_external, config.use_z3_solver)
 
-# Diff command used by Clang Analyzer tests (when comparing .plist files
-# with reference output)
-config.substitutions.append(('%diff_plist',
-'diff -u -w -I "/" -I ".:" -I "version"'))
-
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
Index: test/Analysis/lambda-notes.cpp
===
--- test/Analysis/lambda-notes.cpp
+++ test/Analysis/lambda-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/lambda-notes.cpp.plist 
>%t.expected.sed && echo >>%t && tail -n +11 %t | %normalize_plist | diff -u 
%t.expected.sed -
 
 
 // Diagnostic inside a lambda
Index: test/Analysis/diagnostics/plist-multi-file.c
===
--- test/Analysis/diagnostics/plist-multi-file.c
+++ test/Analysis/diagnostics/plist-multi-file.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/plist-multi-file.c.plist 
>%t.expected.sed.plist && echo >>%t.plist && tail -n +11 %t.plist | 
%normalize_plist | diff -u %t.expected.sed.plist -
 
 #include "plist-multi-file.h"
 
Index: test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
+++ test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: %normalize_plist 
<%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist 
>%t.expected.sed.plist && echo >>%t.plist && tail -n +11 %t.plist | 
%normalize_plist | diff -u %t.expected.sed.plist -
 
 #include 

[PATCH] D59934: Compare SourceLocations from different TUs by FileID

2019-06-06 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

Is it a better fix to change getMacroInfoForLocation?

  static const MacroInfo *getMacroInfoForLocation(const Preprocessor ,
  const SourceManager ,
  const IdentifierInfo *II,
  SourceLocation Loc) {
  
// Can not get macro information for locations not in main TU.
std::pair DecMainLoc =
std::make_pair(SM.getMainFileID(), 0);
std::pair DecLoc = SM.getDecomposedLoc(Loc);
if (!SM.isInTheSameTranslationUnit(DecMainLoc, DecLoc).first)
  return nullptr;
  ...


Repository:
  rC Clang

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

https://reviews.llvm.org/D59934



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


[PATCH] D62950: [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

2019-06-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
hubert.reinterpretcast added a parent revision: D62949: [analyzer][tests] Add 
normalize_plist to replace diff_plist.

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically to 
the cases where the output file is piped to `%diff_plist` via `cat`.

The changes were applied via a script, except that 
`clang/test/Analysis/NewDelete-path-notes.cpp` and 
`clang/test/Analysis/plist-macros-with-expansion.cpp` were each adjusted for 
the line-continuation on the relevant `RUN` step.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62950

Files:
  test/Analysis/NewDelete-path-notes.cpp
  test/Analysis/conditional-path-notes.c
  test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  test/Analysis/copypaste/plist-diagnostics.cpp
  test/Analysis/cxx-for-range.cpp
  test/Analysis/diagnostics/deref-track-symbolic-region.c
  test/Analysis/diagnostics/report-issues-within-main-file.cpp
  test/Analysis/diagnostics/undef-value-caller.c
  test/Analysis/diagnostics/undef-value-param.c
  test/Analysis/diagnostics/undef-value-param.m
  test/Analysis/edges-new.mm
  test/Analysis/generics.m
  test/Analysis/inline-plist.c
  test/Analysis/inline-unique-reports.c
  test/Analysis/inlining/eager-reclamation-path-notes.c
  test/Analysis/inlining/eager-reclamation-path-notes.cpp
  test/Analysis/inlining/path-notes.c
  test/Analysis/inlining/path-notes.cpp
  test/Analysis/inlining/path-notes.m
  test/Analysis/method-call-path-notes.cpp
  test/Analysis/model-file.cpp
  test/Analysis/null-deref-path-notes.m
  test/Analysis/nullability-notes.m
  test/Analysis/objc-arc.m
  test/Analysis/objc-radar17039661.m
  test/Analysis/plist-macros-with-expansion.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/plist-output-alternate.m
  test/Analysis/plist-output.m
  test/Analysis/retain-release-path-notes.m
  test/Analysis/retain-release.m

Index: test/Analysis/retain-release.m
===
--- test/Analysis/retain-release.m
+++ test/Analysis/retain-release.m
@@ -17,8 +17,8 @@
 // RUN: -Wno-objc-root-class -x objective-c++ -std=gnu++98\
 // RUN: -analyzer-config osx.cocoa.RetainCount:TrackNSCFStartParam=true\
 // RUN: -DTRACK_START_PARAM
-// RUN: cat %t.objcpp.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objcpp.plist -
-// RUN: cat %t.objc.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objc.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release.m.objcpp.plist >%t.objcpp.expected.sed.plist && echo >>%t.objcpp.plist && %normalize_plist <%t.objcpp.plist | diff -u %t.objcpp.expected.sed.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release.m.objc.plist >%t.objc.expected.sed.plist && echo >>%t.objc.plist && %normalize_plist <%t.objc.plist | diff -u %t.objc.expected.sed.plist -
 
 void clang_analyzer_eval(int);
 
Index: test/Analysis/retain-release-path-notes.m
===
--- test/Analysis/retain-release-path-notes.m
+++ test/Analysis/retain-release-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=plist-multi-file %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release-path-notes.m.plist >%t.expected.sed && echo >>%t && %normalize_plist <%t | diff -u %t.expected.sed -
 
 /***
 This file is for testing the path-sensitive notes for retain/release errors.
Index: test/Analysis/plist-output.m
===
--- test/Analysis/plist-output.m
+++ test/Analysis/plist-output.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s 

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

2019-06-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html to the specific 
case shown in the list message. Mechanical changes to the other affected files 
will follow in later patches.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62949

Files:
  test/Analysis/lit.local.cfg
  test/Analysis/unix-fns.c


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/unix-fns.c.plist 
>%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist | diff 
-u %t.expected.sed.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/unix-fns.c.plist >%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist | diff -u %t.expected.sed.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50147: clang-format: support external styles

2019-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50147



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


[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2019-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

ping ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50078



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


[PATCH] D37813: clang-format: better handle namespace macros

2019-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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


[clang-tools-extra] r362706 - [clang-tidy] Another attempt to fix misc-redundant-expression check.

2019-06-06 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Thu Jun  6 06:43:38 2019
New Revision: 362706

URL: http://llvm.org/viewvc/llvm-project?rev=362706=rev
Log:
[clang-tidy] Another attempt to fix misc-redundant-expression check.

Correct the fix of rL3627011, the isValueDependent guard was added in a wrong 
place in rL362701.

Modified:
clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp?rev=362706=362705=362706=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp Thu 
Jun  6 06:43:38 2019
@@ -291,7 +291,7 @@ static void transformSubToCanonicalAddEx
 }
 
 AST_MATCHER(Expr, isIntegerConstantExpr) {
-  if (Node.isInstantiationDependent() || Node.isValueDependent())
+  if (Node.isInstantiationDependent())
 return false;
   return Node.isIntegerConstantExpr(Finder->getASTContext());
 }
@@ -523,10 +523,11 @@ static bool retrieveRelationalIntegerCon
 if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
   return false;
 
-if (!OverloadedOperatorExpr->getArg(1)->isIntegerConstantExpr(
-Value, *Result.Context))
-  return false;
-
+if (const auto *Arg = OverloadedOperatorExpr->getArg(1)) {
+  if (!Arg->isValueDependent() &&
+  !Arg->isIntegerConstantExpr(Value, *Result.Context))
+return false;
+}
 Symbol = OverloadedOperatorExpr->getArg(0);
 OperandExpr = OverloadedOperatorExpr;
 Opcode = 
BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());


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


[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-06 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:1005
   bool IsMCUABI;
+  bool IsLinuxABI;
   unsigned DefaultNumRegisterParameters;

Maybe replace the two booleans with something alike `IsPassInMMXRegABI`? And 
while at it, include NetBSD there, please.


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

https://reviews.llvm.org/D59744



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


[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.

Thanks Dmitri!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61487



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


[PATCH] D61486: [Basic] Introduce active dummy DiagnosticBuilder

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.

In D61486#1532272 , @gribozavr wrote:

> Is this patch still needed?


Nope, abandoning it now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61486



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


[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362702: [clang-tidy] Make the plugin honor NOLINT (authored 
by nik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61487

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
  clang-tools-extra/trunk/test/clang-tidy/basic.cpp
  clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
  clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+//  31:7: warning: unused variable 'i' [-Wunused-variable]
+//  int j; // NOLINT
+//  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,48 @@
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
Index: clang-tools-extra/trunk/test/clang-tidy/basic.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/basic.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/basic.cpp
@@ -1,4 +1,5 @@
 // RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang 

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

2019-06-06 Thread Borsik Gábor via Phabricator via cfe-commits
boga95 updated this revision to Diff 203339.

Repository:
  rC Clang

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

https://reviews.llvm.org/D59555

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

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
 char *gets(char *str);
Index: test/Analysis/Inputs/taint-generic-config.yaml
===
--- /dev/null
+++ test/Analysis/Inputs/taint-generic-config.yaml
@@ -0,0 +1,51 @@
+# A list of source/propagation function
+Propagations:
+  # int x = mySource1(); // x is tainted
+  - Name: mySource1
+DstArgs:  [-1] # Index for return value
+
+  # int x;
+  # mySource2(); // x is tainted
+  - Name: mySource2
+DstArgs:  [0]
+
+  # int x, y;
+  # myScanf("%d %d", , ); // x and y are tainted
+  - Name:  myScanf
+VariadicType:  Dst
+VariadicIndex: 1
+
+  # int x; // x is tainted
+  # int y;
+  # myPropagator(x, ); // y is tainted
+  - Name: myPropagator
+SrcArgs:  [0]
+DstArgs:  [1]
+
+  # constexpr unsigned size = 100;
+  # char buf[size];
+  # int x, y;
+  # int n = mySprintf(buf, size, "%d %d", x, y); // If size, x or y is tainted
+  # // the return value and the buf will be tainted
+  - Name:  mySnprintf
+SrcArgs:   [1]
+DstArgs:   [0, -1]
+VariadicType:  Src
+VariadicIndex: 3
+
+# A list of filter functions
+Filters:
+  # int x; // x is tainted
+  # myFilter(); // x is not tainted anymore
+  - Name: myFilter
+Args: [0]
+
+# A list of sink functions
+Sinks:
+  # int x, y; // x and y are tainted
+  # mySink(x, 0, 1); // It will warn
+  # mySink(0, 1, y); // It will warn
+  # mySink(0, x, 1); // It won't warn
+  - Name: mySink
+Args: [0, 2]
+
Index: lib/StaticAnalyzer/Checkers/Yaml.h
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/Yaml.h
@@ -0,0 +1,29 @@
+#include "llvm/ADT/APInt.h"
+#include "llvm/Support/YAMLTraits.h"
+
+/// Read the given file from the filesystem and parse it as a yaml file. The
+/// template parameter must have a yaml MappingTraits.
+template 
+llvm::Optional getConfiguration(llvm::StringRef ConfigFile) {
+  if (ConfigFile.trim().empty())
+return {};
+
+  llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
+  llvm::ErrorOr> Buffer =
+  FS->getBufferForFile(ConfigFile.str());
+
+  if (std::error_code ec = Buffer.getError()) {
+llvm::errs() << "Error when getting TaintPropagation's config file '"
+ << ConfigFile << "': " << ec.message() << '\n';
+return {};
+  }
+
+  llvm::yaml::Input Input(Buffer.get()->getBuffer());
+  T Config;
+  Input >> Config;
+
+  if (std::error_code ec = Input.error())
+return {};
+
+  return Config;
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -15,16 +15,19 @@
 //===--===//
 
 #include "Taint.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
-#include 
-#include 
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 
 using namespace clang;
@@ -44,14 +47,49 @@
 
   void checkPreStmt(const CallExpr 

  1   2   >