[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Eugene Leviant via Phabricator via cfe-commits
evgeny777 added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

tejohnson wrote:
> evgeny777 wrote:
> > tejohnson wrote:
> > > evgeny777 wrote:
> > > > tejohnson wrote:
> > > > > evgeny777 wrote:
> > > > > > I don't think, I understand this.
> > > > > > It looks like that if (a) we have type.test in the module and (b) 
> > > > > > we don't have vtable definition with corresponding type metadata in 
> > > > > > the same module then we'll remove type.test/assume sequence before 
> > > > > > even getting to ICP. This seems strange in the context of previous 
> > > > > > discussion because virtual function may be called in different 
> > > > > > module from one where vtable is defined, like so:
> > > > > > ```
> > > > > > struct A { virtual int foo() { return 42; } };
> > > > > > 
> > > > > > // calls pA->foo
> > > > > > extern int callFoo(A *pA);
> > > > > > int main() { A a; return callFoo(&a); }
> > > > > > ```
> > > > > > 
> > > > > We will only be able to do ICP this way if we have the target vtable 
> > > > > in the module (similar to how we currently can only do ICP if the 
> > > > > target function is in the module). I anticipate doing something 
> > > > > similar for vtable defs as to what we do for virtual functions, where 
> > > > > a fake call edge is added to the summary based on the value profile 
> > > > > results to ensure they are imported before the LTO backend ICP.
> > > > > We will only be able to do ICP this way if we have the target vtable 
> > > > > in the module (similar to how we currently can only do ICP if the 
> > > > > target function is in the module).
> > > > 
> > > > I was thinking of slightly different approach: it seems you have 
> > > > required type information in combined index together with type name in 
> > > > the module, so you probably can add external declarations of required 
> > > > vtables (this may require promotion) to the module in the ICP pass and 
> > > > run ICP over them. Do you think this can work?
> > > Possibly, but we'd still need to have type metadata on those vtable 
> > > declarations, because the type metadata provides the address point offset 
> > > which is needed in the comparison sequence.
> > > Possibly, but we'd still need to have type metadata on those vtable 
> > > declarations, because the type metadata provides the address point offset 
> > > which is needed in the comparison sequence.
> > 
> > I think it's stored in the index in VFuncId structures. Isn't it?
> > I think it's stored in the index in VFuncId structures. Isn't it?
> 
> No, that holds the offset from the address point to the virtual function 
> within the vtable def, not the address point offset itself, which is what we 
> need from the type metadata. But actually, we need both offsets:
> 
> Consider the following example:
> 
> ```
> class A {
>virtual void foo();
> }
> 
> void bar(A *a) {
>a->foo();
> }
> ```
> 
> The indirect call sequence will look like (not showing the type test for 
> simplicity):
> 
> ```
>  %0 = bitcast %class.A* %a to i32 (%class.A*)***
>   %vtable = load i32 (%class.A*)**, i32 (%class.A*)*** %0
>   %1 = load i32 (%class.A*)*, i32 (%class.A*)** %vtable
>   %call = tail call i32 %1(%class.A* %a)
> ```
> 
> With type profiling we will profile the value of %vtable, and figure out that 
> it is associated with the symbol for A's vtable (the profiling support uses 
> symbol table info for this), which is:
> 
> ```
> @_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* 
> null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (i32 
> (%class.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0
> 
> !0 = !{i64 16, !"_ZTS1A"}
> ```
> 
> When we promote the call using the vtable profile results, we need to add the 
> address point offset (16) from the type metadata to the profiled result which 
> will be the symbol @_ZTV1A, before comparing against %vtable.
> 
> We then need to look at the IR and compute the offset to the function address 
> used there (0 in the above IR), and add it to the address point offset, 
> resulting in 16 here, looking in the vtable def to see what function is at 
> the combined offset (_ZN1A3fooEv here), so that we insert a direct call to 
> that function.
> 
> For the address point offset, we only have it in the summary for index-only 
> WPD (TypeIdCompatibleVtableMap). However, I don't want to restrict the ICP 
> improvements to that build mode.
> 
> For the latter, the VFuncId does summarize the offsets, but we can compute it 
> from the IR as I described above (it's the amount added to %vtable before 
> loading the virtual function pointer) and don't need the summary. And in any 
> case the VFuncId doesn't tell us which function is at the aggregate offset, 
> we need the vtable def to tell us that (or the vTableFuncs array saved on the 
> GlobalVarS

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

2020-02-05 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:408
+
+- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage
+  provided for default placement new is sufficiently large.

Szelethus wrote:
> NoQ wrote:
> > I think this one's out of alpha already (i.e., `cplusplus.PlacementNew`). 
> > I'd also bump it to the top because it applies to more users.
> Yup, but AFAIK it moved out of alpha after rc1 was tagged.
`alpha.cplusplus.PlacementNew` not `alpha.plusplus.PlacementNew`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73966



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


[clang-tools-extra] 6e8d6bc - [clangd] Preserve -nostdinc and --sysroot when calling query driver

2020-02-05 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2020-02-05T09:58:06+01:00
New Revision: 6e8d6bc9ec8739ec22b73a23f740f171f452e234

URL: 
https://github.com/llvm/llvm-project/commit/6e8d6bc9ec8739ec22b73a23f740f171f452e234
DIFF: 
https://github.com/llvm/llvm-project/commit/6e8d6bc9ec8739ec22b73a23f740f171f452e234.diff

LOG: [clangd] Preserve -nostdinc and --sysroot when calling query driver

Solves this issue: https://github.com/clangd/clangd/issues/157

This is my first contribution to an llvm project, so I hope I'm doing it right!

Patch by @topisani (Tobias Pisani)!

Reviewers: kadircet, klimek

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

Added: 


Modified: 
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/test/system-include-extractor.test

Removed: 




diff  --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp 
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index e4f5c48544fd..20eb4f8a28e0 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -85,9 +85,10 @@ std::vector parseDriverOutput(llvm::StringRef 
Output) {
   return SystemIncludes;
 }
 
-std::vector extractSystemIncludes(PathRef Driver,
-   llvm::StringRef Lang,
-   llvm::Regex &QueryDriverRegex) {
+std::vector
+extractSystemIncludes(PathRef Driver, llvm::StringRef Lang,
+  llvm::ArrayRef CommandLine,
+  llvm::Regex &QueryDriverRegex) {
   trace::Span Tracer("Extract system includes");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
@@ -120,14 +121,43 @@ std::vector extractSystemIncludes(PathRef 
Driver,
   llvm::Optional Redirects[] = {
   {""}, {""}, llvm::StringRef(StdErrPath)};
 
-  // Should we also preserve flags like "-sysroot", "-nostdinc" ?
-  const llvm::StringRef Args[] = {Driver, "-E", "-x", Lang, "-", "-v"};
+  llvm::SmallVector Args = {Driver, "-E", "-x",
+ Lang,   "-",  "-v"};
+
+  // These flags will be preserved
+  const llvm::StringRef FlagsToPreserve[] = {
+  "-nostdinc", "--no-standard-includes", "-nostdinc++", "-nobuiltininc"};
+  // Preserves these flags and their values, either as separate args or with an
+  // equalsbetween them
+  const llvm::StringRef ArgsToPreserve[] = {"--sysroot", "-isysroot"};
+
+  for (size_t I = 0, E = CommandLine.size(); I < E; ++I) {
+llvm::StringRef Arg = CommandLine[I];
+if (llvm::any_of(FlagsToPreserve,
+ [&Arg](llvm::StringRef S) { return S == Arg; })) {
+  Args.push_back(Arg);
+} else {
+  const auto *Found =
+  llvm::find_if(ArgsToPreserve, [&Arg](llvm::StringRef S) {
+return Arg.startswith(S);
+  });
+  if (Found == std::end(ArgsToPreserve))
+continue;
+  Arg.consume_front(*Found);
+  if (Arg.empty() && I + 1 < E) {
+Args.push_back(CommandLine[I]);
+Args.push_back(CommandLine[++I]);
+  } else if (Arg.startswith("=")) {
+Args.push_back(CommandLine[I]);
+  }
+}
+  }
 
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
  Redirects)) {
 elog("System include extraction: driver execution failed with return code: 
"
- "{0}",
- llvm::to_string(RC));
+ "{0}. Args: ['{1}']",
+ llvm::to_string(RC), llvm::join(Args, "', '"));
 return {};
   }
 
@@ -237,20 +267,18 @@ class QueryDriverDatabase : public 
GlobalCompilationDatabase {
 
 llvm::SmallString<128> Driver(Cmd->CommandLine.front());
 llvm::sys::fs::make_absolute(Cmd->Directory, Driver);
-auto Key = std::make_pair(Driver.str(), Lang);
+auto Key = std::make_pair(Driver.str().str(), Lang.str());
 
 std::vector SystemIncludes;
 {
   std::lock_guard Lock(Mu);
 
-  auto It = DriverToIncludesCache.find(
-  {std::string(Key.first), std::string(Key.second)});
+  auto It = DriverToIncludesCache.find(Key);
   if (It != DriverToIncludesCache.end())
 SystemIncludes = It->second;
   else
-DriverToIncludesCache[{std::string(Key.first),
-   std::string(Key.second)}] = SystemIncludes =
-extractSystemIncludes(Key.first, Key.second, QueryDriverRegex);
+DriverToIncludesCache[Key] = SystemIncludes = extractSystemIncludes(
+Key.first, Key.second, Cmd->CommandLine, QueryDriverRegex);
 }
 
 return addSystemIncludes(*Cmd, SystemIncludes);
@@ -280,7 +308,7 @@ getQueryDriverDatabase(llvm::ArrayRef 
QueryDriverGlobs,
   if (QueryDriverGlobs.empty())
 return Base;
   return std::make_unique(QueryDriverGlobs,
-std::move(Base));
+ 

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-02-05 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio added a comment.

Ping :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932



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


[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

2020-02-05 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:408
+
+- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage
+  provided for default placement new is sufficiently large.

balazske wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > I think this one's out of alpha already (i.e., `cplusplus.PlacementNew`). 
> > > I'd also bump it to the top because it applies to more users.
> > Yup, but AFAIK it moved out of alpha after rc1 was tagged.
> `alpha.cplusplus.PlacementNew` not `alpha.plusplus.PlacementNew`?
`cplusplus.PlacementNew`
(cf. https://github.com/llvm/llvm-project/commit/bc29069dc40)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73966



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


[PATCH] D74025: [clangd] Add the missing elaborated types in FindTarget.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74025

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -241,6 +241,13 @@
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
+  Code = R"cpp(
+namespace ns { struct S{}; }
+typedef ns::S X;
+[[X]] x;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
+   {"struct S", Rel::Underlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -379,6 +379,10 @@
 Outer.add(TT->getAsTagDecl(), Flags);
   }
 
+  void VisitElaboratedType(const ElaboratedType* ET) {
+Outer.add(ET->desugar(), Flags);
+  }
+
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT) {
 Outer.add(ICNT->getDecl(), Flags);
   }


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -241,6 +241,13 @@
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
+  Code = R"cpp(
+namespace ns { struct S{}; }
+typedef ns::S X;
+[[X]] x;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
+   {"struct S", Rel::Underlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -379,6 +379,10 @@
 Outer.add(TT->getAsTagDecl(), Flags);
   }
 
+  void VisitElaboratedType(const ElaboratedType* ET) {
+Outer.add(ET->desugar(), Flags);
+  }
+
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT) {
 Outer.add(ICNT->getDecl(), Flags);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73453: Preserve -nostdinc and --sysroot when calling query driver

2020-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet closed this revision.
kadircet added a comment.

closed by 6e8d6bc9ec8739ec22b73a23f740f171f452e234 



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

https://reviews.llvm.org/D73453



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


[PATCH] D74025: [clangd] Add the missing elaborated types in FindTarget.

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62441 tests passed, 0 failed 
and 845 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74025



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


[clang-tools-extra] ca1c21d - [clangd] Use printf instead of `echo -e` to be compliant with dash

2020-02-05 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2020-02-05T10:21:32+01:00
New Revision: ca1c21d4b659bfa5edb38c4a54b3050e43c4b51a

URL: 
https://github.com/llvm/llvm-project/commit/ca1c21d4b659bfa5edb38c4a54b3050e43c4b51a
DIFF: 
https://github.com/llvm/llvm-project/commit/ca1c21d4b659bfa5edb38c4a54b3050e43c4b51a.diff

LOG: [clangd] Use printf instead of `echo -e` to be compliant with dash

Added: 


Modified: 
clang-tools-extra/clangd/test/system-include-extractor.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/system-include-extractor.test 
b/clang-tools-extra/clangd/test/system-include-extractor.test
index b1551d8f8ed3..74d395869ad0 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -12,10 +12,10 @@
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> 
%t.dir/my_driver.sh
 # RUN: echo 'echo " $* " | grep " --sysroot /my/sysroot/path " || exit' >> 
%t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
-# RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> 
%t.dir/my_driver.sh
+# RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> 
%t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/my_driver.sh
-# RUN: echo 'echo -e "End of search list.\r" >&2' >> %t.dir/my_driver.sh
+# RUN: echo 'printf "End of search list.\r\n" >&2' >> %t.dir/my_driver.sh
 # RUN: chmod +x %t.dir/my_driver.sh
 
 # Create header files my/dir/a.h and my/dir2/b.h



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


[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-02-05 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932



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


[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

thanks, mostly good. the only concern from my side is about the 
`SymbolRole::Implicit`, I think we should get rid of it.




Comment at: clang-tools-extra/clangd/index/Ref.h:53
+  // The reference explicitly spells out declaration's name. Such references 
can
+  // not come from macro expansions or implicit AST nodes.
+  //

maybe make sense to include an AST example?

```
class Foo { public: Foo(); };

Foo [[f]]; // there is a reference of `Foo` constructor around `f`, this is a 
non-spelled reference obviously.
```



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:191
+Result |= RefKind::Reference;
+  if (!(Roles & static_cast(index::SymbolRole::Implicit)))
+Result |= RefKind::Spelled;

kbobyrev wrote:
> hokein wrote:
> > I was confused at the first time reading the code -- it turns out we reset 
> > the flag in the caller. 
> > 
> > Maybe adjust the function like `RefKind toRefKind(index::SymbolRoleSet 
> > Roles, bool isSpelled)`?
> If my understanding is correct, the suggestion is to take `isSpelled` 
> argument and apply the `RefKind` flag based on the value of the argument 
> instead of using `SymbolRole::Implicit`. Is that right? In this case I would 
> be concerned about doing because that increase the amount of code in case 
> there is any other index provider that sets the flags itself. 
> 
> What do you think?
it is not about the amount of code, it is about layering. I think we should not 
use the `Implicit`.  With the current change, the `SymbolRole::Implicit` comes 
from two sources:

1) the flag can be set in libindex;
2) the flag can be set in SymbolCollector (via the post-processing in `finish`)

for 1), the implementation of libindex is not completed, only object-c related 
decls get set. I think our aim is to set the `Spelled` flag if 2) is true.


Another way doing that would be keep the `toRefKind` implementation unchanged, 
and set the `Spelled` flag afterwards.



Comment at: clang-tools-extra/clangd/index/SymbolLocation.h:12
 
+#include "Protocol.h"
 #include "llvm/ADT/StringRef.h"

I would not do this change, it introduces a new dependency to this header.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:36
   Ref Result;
-  Result.Kind = RefKind::Reference;
+  Result.Kind = RefKind::Reference | RefKind::Spelled;
   Result.Location.Start.setLine(Range.start.line);

do we need this change in this patch? 



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:730
+  AllOf(HaveRanges(AllRanges),
+RefKindIs(RefKind::Spelled, SpelledRanges);
+}

I found the RefKindIs is hard to follow without reading its implementation, I 
think we can do it in another way, retrieve all spelled refs from `Refs`, and 
verify them:

```
EXPECT_THAT(SpelledRefs,
 UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID,
   
AllOf(HaveRanges(Main.ranges("spelled"))), SpelledKind()));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746



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


[PATCH] D74028: [clang] Add release notes for the 10.x branch for things I've done

2020-02-05 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo created this revision.
mstorsjo added reviewers: rnk, hans.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74028

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -194,6 +194,12 @@
 
 error LNK2005: "bool const std::_Is_integral" 
(??$_Is_integral@H@std@@3_NB) already defined
 
+- The ``.exe`` output suffix is now added implicitly in MinGW mode, when
+  Clang is running on Windows (matching GCC's behaviour)
+
+- Fixed handling of TLS variables that are shared between object files
+  in MinGW environments
+
 C Language Changes in Clang
 ---
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -194,6 +194,12 @@
 
 error LNK2005: "bool const std::_Is_integral" (??$_Is_integral@H@std@@3_NB) already defined
 
+- The ``.exe`` output suffix is now added implicitly in MinGW mode, when
+  Clang is running on Windows (matching GCC's behaviour)
+
+- Fixed handling of TLS variables that are shared between object files
+  in MinGW environments
+
 C Language Changes in Clang
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780



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


[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 242536.
hokein added a comment.

update based on the offline discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -1127,5 +1127,19 @@
   return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
 }
 
+bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
+  auto FileName = SM.getFilename(Loc);
+  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+return false;
+  auto FID = SM.getFileID(Loc);
+  // All proto generated headers should start with this line.
+  static const char *PROTO_HEADER_COMMENT =
+  "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
+  // Double check that this is an actual protobuf header.
+  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+return false;
+  return true;
+}
+
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2020-02-05 Thread Thibault North via Phabricator via cfe-commits
tnorth added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D72326



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Andi via Phabricator via cfe-commits
Abpostelnicu created this revision.
Abpostelnicu added a reviewer: hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Abpostelnicu edited the summary of this revision.
Abpostelnicu edited the summary of this revision.
Abpostelnicu added a subscriber: sylvestre.ledru.

Since D66404   adds some significant 
modifications to the `CFG` we should include it in the release notes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74031

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,8 @@
 libclang
 
 
-- ...
+- Various changes to reduce discrepancies in destructor calls between the
+  generated ``CFG`` and the actual ``codegen``.
 
 Static Analyzer
 ---


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,8 @@
 libclang
 
 
-- ...
+- Various changes to reduce discrepancies in destructor calls between the
+  generated ``CFG`` and the actual ``codegen``.
 
 Static Analyzer
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang-tools-extra/clangd/SourceCode.cpp:1139
+  // Double check that this is an actual protobuf header.
+  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+return false;

nit: just `return SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780



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


[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:408
+
+- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage
+  provided for default placement new is sufficiently large.

Charusso wrote:
> balazske wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > I think this one's out of alpha already (i.e., 
> > > > `cplusplus.PlacementNew`). I'd also bump it to the top because it 
> > > > applies to more users.
> > > Yup, but AFAIK it moved out of alpha after rc1 was tagged.
> > `alpha.cplusplus.PlacementNew` not `alpha.plusplus.PlacementNew`?
> `cplusplus.PlacementNew`
> (cf. https://github.com/llvm/llvm-project/commit/bc29069dc40)
Nice catch ;) I checked out the `llvmorg-10.0.0-rc1` tag and can confirm that 
at that point, the checker is still in alpha, unfortunately.


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

https://reviews.llvm.org/D73966



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


[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 242539.
Szelethus marked 10 inline comments as done.
Szelethus retitled this revision from "[analyzer][WIP] Add 10.0.0 release 
notes." to "[analyzer] Add 10.0.0 release notes.".

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

https://reviews.llvm.org/D73966

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -402,10 +402,31 @@
 Static Analyzer
 ---
 
+- New checker: ``fuchsia.HandleChecker`` to detect leaks related to Fuchsia
+  handles.
+
+- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage
+  provided for default placement new is sufficiently large.
+
 - The Clang analyzer checker ``DeadStores`` gets a new option called
   ``WarnForDeadNestedAssignments`` to detect nested dead assignments
   (enabled by default).
-- ...
+
+- Condition values that greatly affect the occurance of a bug are now far 
better
+  explained in bug reports (further reading on the related
+  `GSoC'19 summary page `_).
+
+- Despite still in being in alpha stage, checkers implementing taint analyses
+  and C++ iterator rules were improved greatly.
+
+- Analyses on LLVM's own source code are far more precise due to the modeling 
of
+  several LLVM specific techniques, like its custom RTTI, informing the 
analyzer
+  of the return values of core functions, and much more (further reading on the
+  related `GSoC'19 summary page 
`_)
 .
+
+- ObjectiveC++ changes:
+
+- Numerous smaller false positive fixes.
 
 .. _release-notes-ubsan:
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -402,10 +402,31 @@
 Static Analyzer
 ---
 
+- New checker: ``fuchsia.HandleChecker`` to detect leaks related to Fuchsia
+  handles.
+
+- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage
+  provided for default placement new is sufficiently large.
+
 - The Clang analyzer checker ``DeadStores`` gets a new option called
   ``WarnForDeadNestedAssignments`` to detect nested dead assignments
   (enabled by default).
-- ...
+
+- Condition values that greatly affect the occurance of a bug are now far better
+  explained in bug reports (further reading on the related
+  `GSoC'19 summary page `_).
+
+- Despite still in being in alpha stage, checkers implementing taint analyses
+  and C++ iterator rules were improved greatly.
+
+- Analyses on LLVM's own source code are far more precise due to the modeling of
+  several LLVM specific techniques, like its custom RTTI, informing the analyzer
+  of the return values of core functions, and much more (further reading on the
+  related `GSoC'19 summary page `_) .
+
+- ObjectiveC++ changes:
+
+- Numerous smaller false positive fixes.
 
 .. _release-notes-ubsan:
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 242543.
hokein added a comment.

address a final comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -1127,5 +1127,17 @@
   return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
 }
 
+bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
+  auto FileName = SM.getFilename(Loc);
+  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+return false;
+  auto FID = SM.getFileID(Loc);
+  // All proto generated headers should start with this line.
+  static const char *PROTO_HEADER_COMMENT =
+  "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
+  // Double check that this is an actual protobuf header.
+  return SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT);
+}
+
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+
 } // namespace clangd
 } // namespace clang
 

[PATCH] D74033: [clang-tidy] Fix PR#44620 'readability-redundant-string-cstr quick-fix causes invalid code'

2020-02-05 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, xazax.hun.

  #include 
  
  static void f2(std::string&&) {
  }
  
  static void f() {
std::string const s;
f2(s.c_str()); // readability-redundant-string-cstr warning
  }

For this case I decide to do nothing by skipping it in the matcher. Another way 
is to fix with 'std::move(s)' but I`m not sure that it is correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74033

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
@@ -205,3 +205,9 @@
 void invalid(const NotAString &s) {
   dummy(s.c_str());
 }
+
+// Test for rvalue std::string
+
+void m1(std::string&& s) {
+  m1(s.c_str());
+}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -61,6 +61,54 @@
   return (llvm::Twine("*") + Text).str();
 }
 
+// Trying to get CallExpr in which CxxConstructExpr is called
+const clang::CallExpr *
+tryGetCallExprAncestorForCxxConstructExpr(const Expr *TheExpr,
+ ASTContext &Context) {
+  // We skip nodes such as CXXBindTemporaryExpr, MaterializeTemporaryExpr
+  for (ast_type_traits::DynTypedNode DynParent : Context.getParents(*TheExpr)) 
{
+if (const auto *Parent = DynParent.get()) {
+  if (const auto *TheCallExpr = dyn_cast(Parent))
+return TheCallExpr;
+
+  if (const clang::CallExpr *TheCallExpr =
+  tryGetCallExprAncestorForCxxConstructExpr(Parent, Context))
+return TheCallExpr;
+}
+  }
+
+  return nullptr;
+}
+
+// Check that ParamDecl of CallExprDecl has rvalue type
+bool checkParamDeclOfAncestorCallExprHasRValueRefType(
+const Expr *TheCxxConstructExpr, ASTContext &Context) {
+  if (const clang::CallExpr *TheCallExpr =
+  tryGetCallExprAncestorForCxxConstructExpr(TheCxxConstructExpr,
+   Context)) {
+for (int i = 0; i < TheCallExpr->getNumArgs(); ++i) {
+  const Expr *Arg = TheCallExpr->getArg(i);
+  if (Arg->getSourceRange() == TheCxxConstructExpr->getSourceRange()) {
+if (const auto *TheCallExprDecl =
+dyn_cast(TheCallExpr->getCalleeDecl())) {
+  if (TheCallExprDecl->getParamDecl(i)
+  ->getType()
+  ->isRValueReferenceType())
+return true;
+}
+  }
+}
+  }
+
+  return false;
+}
+
+AST_MATCHER(CXXConstructExpr,
+matchedParamDeclOfAncestorCallExprHasRValueRefType) {
+  return checkParamDeclOfAncestorCallExprHasRValueRefType(
+  &Node, Finder->getASTContext());
+}
+
 } // end namespace
 
 void RedundantStringCStrCheck::registerMatchers(
@@ -95,9 +143,13 @@
   .bind("call");
 
   // Detect redundant 'c_str()' calls through a string constructor.
-  Finder->addMatcher(cxxConstructExpr(StringConstructorExpr,
-  hasArgument(0, StringCStrCallExpr)),
- this);
+  // If CxxConstructExpr is the part of some CallExpr we need to
+  // check that matched ParamDecl of the ancestor CallExpr is not rvalue
+  Finder->addMatcher(
+  cxxConstructExpr(
+  StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+  unless(matchedParamDeclOfAncestorCallExprHasRValueRefType())),
+  this);
 
   // Detect: 's == str.c_str()'  ->  's == str'
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp
@@ -205,3 +205,9 @@
 void invalid(const NotAString &s) {
   dummy(s.c_str());
 }
+
+// Test for rvalue std::string
+
+void m1(std::string&& s) {
+  m1(s.c_str());
+}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -61,6 +61,54 @@
   return (llvm::Twine("*") + Text).str();
 }
 
+// Trying to get C

[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780



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


[PATCH] D73693: [clang][DeclPrinter] Implement visitors for {TemplateType,NonTypeTemplate}Parms

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

if it is trivial, it would be nice to add some unittests in 
`clang/unittests/AST/DeclPrinterTest.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73693



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


[clang-tools-extra] f8865c0 - [clangd] Pull out a isProtoFile function.

2020-02-05 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2020-02-05T12:04:03+01:00
New Revision: f8865c01944fe409857eeb30e0963bfa5ced294d

URL: 
https://github.com/llvm/llvm-project/commit/f8865c01944fe409857eeb30e0963bfa5ced294d
DIFF: 
https://github.com/llvm/llvm-project/commit/f8865c01944fe409857eeb30e0963bfa5ced294d.diff

LOG: [clangd] Pull out a isProtoFile function.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: merge_guards_bot, mgorny, ilya-biryukov, MaskRay, jkorous, 
arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/index/SymbolCollector.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SourceCode.cpp 
b/clang-tools-extra/clangd/SourceCode.cpp
index 67cea92962a1..5ac738055b54 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1127,5 +1127,17 @@ bool isHeaderFile(llvm::StringRef FileName,
   return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
 }
 
+bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
+  auto FileName = SM.getFilename(Loc);
+  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+return false;
+  auto FID = SM.getFileID(Loc);
+  // All proto generated headers should start with this line.
+  static const char *PROTO_HEADER_COMMENT =
+  "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
+  // Double check that this is an actual protobuf header.
+  return SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT);
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/SourceCode.h 
b/clang-tools-extra/clangd/SourceCode.h
index 47fde505c252..112f217df134 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@ llvm::Optional locateMacroAt(SourceLocation 
Loc,
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+
 } // namespace clangd
 } // namespace clang
 #endif

diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp 
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 48c26bae5c2c..8718b7bcd48f 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@ std::string toURI(const SourceManager &SM, llvm::StringRef 
Path,
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.



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


[PATCH] D73780: [clangd] Separate protobuf-related functions to a dedicated file.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8865c01944f: [clangd] Pull out a isProtoFile function. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73780

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -1127,5 +1127,17 @@
   return Lang != types::TY_INVALID && types::onlyPrecompileType(Lang);
 }
 
+bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
+  auto FileName = SM.getFilename(Loc);
+  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
+return false;
+  auto FID = SM.getFileID(Loc);
+  // All proto generated headers should start with this line.
+  static const char *PROTO_HEADER_COMMENT =
+  "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
+  // Double check that this is an actual protobuf header.
+  return SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT);
+}
+
 } // namespace clangd
 } // namespace clang


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -70,25 +70,13 @@
   return URI::create(AbsolutePath).toString();
 }
 
-// All proto generated headers should start with this line.
-static const char *PROTO_HEADER_COMMENT =
-"// Generated by the protocol buffer compiler.  DO NOT EDIT!";
-
 // Checks whether the decl is a private symbol in a header generated by
 // protobuf compiler.
-// To identify whether a proto header is actually generated by proto compiler,
-// we check whether it starts with PROTO_HEADER_COMMENT.
 // FIXME: make filtering extensible when there are more use cases for symbol
 // filters.
 bool isPrivateProtoDecl(const NamedDecl &ND) {
   const auto &SM = ND.getASTContext().getSourceManager();
-  auto Loc = nameLocation(ND, SM);
-  auto FileName = SM.getFilename(Loc);
-  if (!FileName.endswith(".proto.h") && !FileName.endswith(".pb.h"))
-return false;
-  auto FID = SM.getFileID(Loc);
-  // Double check that this is an actual protobuf header.
-  if (!SM.getBufferData(FID).startswith(PROTO_HEADER_COMMENT))
+  if (!isProtoFile(nameLocation(ND, SM), SM))
 return false;
 
   // ND without identifier can be operators.
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,9 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Returns true if the given location is in a generated protobuf file.
+bool isProtoFile(SourceLocation Loc, const

[PATCH] D73696: [clang][Index] Introduce a TemplateParm SymbolKind

2020-02-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Libclang changes need tests I think.




Comment at: clang-tools-extra/clangd/Protocol.cpp:265
   case index::SymbolKind::Parameter:
+  case index::SymbolKind::TemplateParm:
 return SymbolKind::Variable;

this seems kind of dubious, maybe worth a comment?
(If we had the decl here we'd distinguish between type and non-type, right?)



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:586
+ HI.LocalScope = "foo::";
+   }},
+  {// TemplateTemplate Type Parameter

I do think we're going to want to have `HI.Type = "typename"` here, maybe with 
some special-case in the rendering.

(Especially with concepts where the TTP can be constrained as Integral or 
something)

Fine to leave for now but FIXME somewhere?



Comment at: clang/include/clang-c/Index.h:6257
+  CXIdxEntity_CXXInterface  = 26,
+  CXIdxEntity_CXXTemplateParm   = 27
 

As far as hover goes, calling this "template parameter" and making int/class 
the "type" is cute and fits well.

But I worry we're not exposing enough info here and at the libclang level (and 
I guess Index) we should be distinguishing the 3 cases. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73696



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


[PATCH] D74036: [clangd] don't rename on protobuf symbols.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74036

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -630,6 +630,21 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, ProtobufSymbolIsBlacklisted) {
+  Annotations Code("Prot^obuf buf;");
+  auto TU = TestTU::withCode(Code.code());
+  TU.HeaderCode =
+  R"cpp(// Generated by the protocol buffer compiler.  DO NOT EDIT!
+  class Protobuf {};
+  )cpp";
+  TU.HeaderFilename = "protobuf.pb.h";
+  auto AST = TU.build();
+  auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results);
+  EXPECT_THAT(llvm::toString(Results.takeError()),
+  testing::HasSubstr("not a supported kind"));
+}
+
 TEST(CrossFileRenameTests, DirtyBuffer) {
   Annotations FooCode("class [[Foo]] {};");
   std::string FooPath = testPath("foo.cc");
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -97,6 +97,9 @@
 }
 
 bool isBlacklisted(const NamedDecl &RenameDecl) {
+  if (isProtoFile(RenameDecl.getLocation(),
+  RenameDecl.getASTContext().getSourceManager()))
+return true;
   static const auto *StdSymbols = new llvm::DenseSet({
 #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
 #include "StdSymbolMap.inc"


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -630,6 +630,21 @@
 expectedResult(Code, NewName));
 }
 
+TEST(RenameTest, ProtobufSymbolIsBlacklisted) {
+  Annotations Code("Prot^obuf buf;");
+  auto TU = TestTU::withCode(Code.code());
+  TU.HeaderCode =
+  R"cpp(// Generated by the protocol buffer compiler.  DO NOT EDIT!
+  class Protobuf {};
+  )cpp";
+  TU.HeaderFilename = "protobuf.pb.h";
+  auto AST = TU.build();
+  auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename)});
+  EXPECT_FALSE(Results);
+  EXPECT_THAT(llvm::toString(Results.takeError()),
+  testing::HasSubstr("not a supported kind"));
+}
+
 TEST(CrossFileRenameTests, DirtyBuffer) {
   Annotations FooCode("class [[Foo]] {};");
   std::string FooPath = testPath("foo.cc");
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -97,6 +97,9 @@
 }
 
 bool isBlacklisted(const NamedDecl &RenameDecl) {
+  if (isProtoFile(RenameDecl.getLocation(),
+  RenameDecl.getASTContext().getSourceManager()))
+return true;
   static const auto *StdSymbols = new llvm::DenseSet({
 #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
 #include "StdSymbolMap.inc"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74036: [clangd] don't rename on protobuf symbols.

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74036



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


[clang] 6198e1c - Fix MSVC signed/unsigned warning. NFCI.

2020-02-05 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2020-02-05T11:53:16Z
New Revision: 6198e1c40abce7135ce68e2b91d433ec02454a50

URL: 
https://github.com/llvm/llvm-project/commit/6198e1c40abce7135ce68e2b91d433ec02454a50
DIFF: 
https://github.com/llvm/llvm-project/commit/6198e1c40abce7135ce68e2b91d433ec02454a50.diff

LOG: Fix MSVC signed/unsigned warning. NFCI.

Added: 


Modified: 
clang/lib/AST/Expr.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 0b6b1e39183c..ac4fa127af43 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -320,7 +320,7 @@ ConstantExpr *ConstantExpr::CreateEmpty(const ASTContext 
&Context,
 }
 
 void ConstantExpr::MoveIntoResult(APValue &Value, const ASTContext &Context) {
-  assert(getStorageKind(Value) <= ConstantExprBits.ResultKind &&
+  assert((unsigned)getStorageKind(Value) <= ConstantExprBits.ResultKind &&
  "Invalid storage for this value kind");
   ConstantExprBits.APValueKind = Value.getKind();
   switch (ConstantExprBits.ResultKind) {



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


[clang] f780e15 - [OpenCL] Fix support for cl_khr_mipmap_image_writes

2020-02-05 Thread Alexey Sotkin via cfe-commits

Author: Alexey Sotkin
Date: 2020-02-05T14:55:32+03:00
New Revision: f780e15caf1bed0a9fbc87fde70bd5ab3d80a439

URL: 
https://github.com/llvm/llvm-project/commit/f780e15caf1bed0a9fbc87fde70bd5ab3d80a439
DIFF: 
https://github.com/llvm/llvm-project/commit/f780e15caf1bed0a9fbc87fde70bd5ab3d80a439.diff

LOG: [OpenCL] Fix support for cl_khr_mipmap_image_writes

Text of the extension is available here:
https://github.com/KhronosGroup/OpenCL-Docs/blob/master/ext/cl_khr_mipmap_image.asciidoc

Patch by Ilya Mashkov

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

Added: 


Modified: 
clang/include/clang/Basic/OpenCLExtensions.def
clang/lib/Headers/opencl-c.h
clang/test/SemaOpenCL/extension-version.cl

Removed: 




diff  --git a/clang/include/clang/Basic/OpenCLExtensions.def 
b/clang/include/clang/Basic/OpenCLExtensions.def
index 5536a6e8e4df..517481584313 100644
--- a/clang/include/clang/Basic/OpenCLExtensions.def
+++ b/clang/include/clang/Basic/OpenCLExtensions.def
@@ -70,6 +70,7 @@ OPENCLEXT_INTERNAL(cl_khr_spir, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)

diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 06c5ab6a72f0..3210f93cc851 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -14682,7 +14682,7 @@ void __ovld write_imagef(write_only 
image2d_array_depth_t image, int4 coord, flo
 
 // OpenCL Extension v2.0 s9.18 - Mipmaps
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image1d_t image, int coord, int lod, 
uint4 color);
@@ -14699,15 +14699,16 @@ void __ovld write_imagef(write_only image2d_array_t 
image_array, int4 coord, int
 void __ovld write_imagei(write_only image2d_array_t image_array, int4 coord, 
int lod, int4 color);
 void __ovld write_imageui(write_only image2d_array_t image_array, int4 coord, 
int lod, uint4 color);
 
-void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float color);
-void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float color);
+void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float depth);
+void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float depth);
 
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imagef(write_only image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#endif //defined(cl_khr_mipmap_image_writes)
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type
@@ -14756,7 +14757,7 @@ void __ovld write_imagef(read_write 
image2d_array_depth_t image, int4 coord, flo
 #endif //cl_khr_depth_images
 
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
 void __ovld write_imagef(read_write image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image1d_t image, int coord, int lod, 
uint4 color);
@@ -14780,8 +14781,9 @@ void __ovld write_imagef(read_write 
image2d_array_depth_t image, int4 coord, int
 void __ovld write_imagef(read_write image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#endif //cl_khr_mipmap_image_writes
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type

diff  --git a/clang/test/SemaOpenCL/extension-version.cl 
b/clang/test/SemaOpenCL/extension-version.cl
index 19d088495350..0e6bbb7d3bcd 100644
--- a/clang/test/SemaOpenCL/extension-version.cl
+++ b/clang/test/SemaOpenCL/extension-version.cl
@@ -242,6 +242,18 @@
 #endif
 #pragma OPENCL EXTENSION cl_khr_m

[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

2020-02-05 Thread Alexey Sotkin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
AlexeySotkin marked an inline comment as done.
Closed by commit rGf780e15caf1b: [OpenCL] Fix support for 
cl_khr_mipmap_image_writes (authored by AlexeySotkin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71460

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Headers/opencl-c.h
  clang/test/SemaOpenCL/extension-version.cl


Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -243,6 +243,18 @@
 #pragma OPENCL EXTENSION cl_khr_mipmap_image : enable
 
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+#ifndef cl_khr_mipmap_image_writes
+#error "Missing cl_khr_mipmap_image_writes define"
+#endif
+#else
+#ifdef cl_khr_mipmap_image_writes
+#error "Incorrect cl_khr_mipmap_image_writes define"
+#endif
+// expected-warning@+2{{unsupported OpenCL extension 
'cl_khr_mipmap_image_writes' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : enable
+
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #ifndef cl_khr_srgb_image_writes
 #error "Missing cl_khr_srgb_image_writes define"
 #endif
Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -14682,7 +14682,7 @@
 
 // OpenCL Extension v2.0 s9.18 - Mipmaps
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image1d_t image, int coord, int lod, 
uint4 color);
@@ -14699,15 +14699,16 @@
 void __ovld write_imagei(write_only image2d_array_t image_array, int4 coord, 
int lod, int4 color);
 void __ovld write_imageui(write_only image2d_array_t image_array, int4 coord, 
int lod, uint4 color);
 
-void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float color);
-void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float color);
+void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float depth);
+void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float depth);
 
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imagef(write_only image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#endif //defined(cl_khr_mipmap_image_writes)
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type
@@ -14756,7 +14757,7 @@
 #endif //cl_khr_depth_images
 
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
 void __ovld write_imagef(read_write image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image1d_t image, int coord, int lod, 
uint4 color);
@@ -14780,8 +14781,9 @@
 void __ovld write_imagef(read_write image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#endif //cl_khr_mipmap_image_writes
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type
Index: clang/include/clang/Basic/OpenCLExtensions.def
===
--- clang/include/clang/Basic/OpenCLExtensions.def
+++ clang/include/clang/Basic/OpenCLExtensions.def
@@ -70,6 +70,7 @@
 OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)


Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/e

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

Apologies for inserting myself in here :)

Please use the "[analyzer]" tag instead of "[clang][checkers]" in future 
changes, because we've used that traditionally for years, and most of us are 
automatically subscribed to it. The term "checker" is mostly known for these 
modules in the analyzer, but I'm pretty sure its used in many other places, 
considering that the main selling point of clang is "checking" the source code 
better then most other compilers.

---

You stated that

> First simple implementation, infrastructure is ready.

Yet the checker code is still about as long as in D71510 
. I share the sentiment of @NoQ in 
D71510#1818438 , this is far too big 
to review thoroughly, even if we're putting it in alpha first and leave the 
gritty bits for later. Despite the patch's aim is to introduce an 
infrastructure, I'm reading code about function filters, special cases for 
variadic functions, explicit casts to void, 3 different classes for 3 different 
kinds of return values, sometimes we "save" arguments, but not always, and many 
other things I failed to understand.

I like to pinpoint to this comment that explains well why we historically 
preferred smaller patches in the analyzer: D45532#1083670 
 (which is on, ironically, my patch).

I see that you're using a statement visitor, which always raises the question 
as to why wasn't a matcher sufficient. Although, I'm not exactly sure why we're 
using a syntactic check for a problem so inherently pathsensitive at all, and I 
couldn't really find an explanation in the code.

---

I'm not sure whether we want to hunt cases like this down, or at least not with 
this checker:

  void test_Null_UnusedVar() {
void *P = aligned_alloc(4, 8);
  } // expected-warning{{Return value was not checked}}

Isn't this a dead store or unused variable issue? Or, in this specific case, 
also a leak.

---

There were valid questions raised by D71510#1818438 
, and very few of those were 
addressed. I tried looking up discussions on discord, cfe-dev, internal mailing 
lists, the previous patch and I didn't find much. I would prefer to first agree 
on what we're shooting for and how, and have a battle plan drawn and explained 
in the code, such as

1. We're sifting out the functions whose return value should always be checked.
2. Should the function be of interest, we're adding its return value to the GDM.
3. Do some analysis on whether the return value was validated.
  1. For this, we're using syntactic analysis (?), which starts with ...
  2. If ... then ...
  3. etc
4. Report errors if so.

The test cases convince me that there is real value to your work, but I'd urge 
you to take a few steps back and leave room for discussion, and reduce the 
scope of this patch to only establish the infrastructure, and maybe find the 
simplest case. I can't stress enough that the idea is great, and even if this 
ends up as an optin checker we should have it, so please don't take my stance 
as anything else but caution :)




Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:44
+/// as error check.
+class CheckForErrorResultChecker {
+public:

Please avoid calling any class a checker if they are not inheriting from 
`Checker<>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705



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


[PATCH] D73350: [analyzer] Small StreamChecker refactoring (NFC).

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

Cool!




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:157
+  ProgramStateRef StateNotNull, StateNull;
+  std::tie(StateNotNull, StateNull) = 
C.getConstraintManager().assumeDual(State, RetVal);
 

This line is more then 80 columns, could you please run `clang-format-diff.py`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73350



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


[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2020-02-05 Thread Jaydeep Chauhan via Phabricator via cfe-commits
Jac1494 added a comment.

ping


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

https://reviews.llvm.org/D71451



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

lgtm, but if you want it wouldn't hurt to include even more details from the 
commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031



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


[PATCH] D73359: [analyzer]StreamChecker refactoring (NFC).

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Charusso.

LGTM! The code looks a lot cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73359



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Andi via Phabricator via cfe-commits
Abpostelnicu added a comment.

In D74031#1859084 , @hans wrote:

> lgtm, but if you want it wouldn't hurt to include even more details from the 
> commit message.


This is a good idea, in my latest patch you will see that I've added more 
context present in D66404 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Andi via Phabricator via cfe-commits
Abpostelnicu updated this revision to Diff 242562.
Abpostelnicu added a comment.

Adding extra comments to the release patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,24 @@
 libclang
 
 
-- ...
+- Various changes to reduce discrepancies in destructor calls between the
+  generated ``CFG`` and the actual ``codegen``.
+
+  In particular:
+
+  - Respect C++17 copy elision; previously it would generate destructor calls
+for elided temporaries, including in initialization and return statements.
+
+  - Don't generate duplicate destructor calls for statement expressions.
+
+  - Fix initialization lists.
+
+  - Fix comma operator.
+
+  - Change printing of implicit destructors to print the type instead of the
+class name directly, matching the code for temporary object destructors.
+The class name was blank for lambdas.
+
 
 Static Analyzer
 ---


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,24 @@
 libclang
 
 
-- ...
+- Various changes to reduce discrepancies in destructor calls between the
+  generated ``CFG`` and the actual ``codegen``.
+
+  In particular:
+
+  - Respect C++17 copy elision; previously it would generate destructor calls
+for elided temporaries, including in initialization and return statements.
+
+  - Don't generate duplicate destructor calls for statement expressions.
+
+  - Fix initialization lists.
+
+  - Fix comma operator.
+
+  - Change printing of implicit destructors to print the type instead of the
+class name directly, matching the code for temporary object destructors.
+The class name was blank for lambdas.
+
 
 Static Analyzer
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] eaf0c89 - [clangd] Add the missing elaborated types in FindTarget.

2020-02-05 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2020-02-05T14:03:36+01:00
New Revision: eaf0c89ec5f866b6cef296c542c030bb2cf8481d

URL: 
https://github.com/llvm/llvm-project/commit/eaf0c89ec5f866b6cef296c542c030bb2cf8481d
DIFF: 
https://github.com/llvm/llvm-project/commit/eaf0c89ec5f866b6cef296c542c030bb2cf8481d.diff

LOG: [clangd] Add the missing elaborated types in FindTarget.

Reviewers: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 13e4819bec9b..a9ac4b86d665 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -379,6 +379,10 @@ struct TargetFinder {
 Outer.add(TT->getAsTagDecl(), Flags);
   }
 
+  void VisitElaboratedType(const ElaboratedType *ET) {
+Outer.add(ET->desugar(), Flags);
+  }
+
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT) {
 Outer.add(ICNT->getDecl(), Flags);
   }

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 22aeed6f5272..074a24d9edbf 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -241,6 +241,13 @@ TEST_F(TargetDeclTest, Types) {
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
+  Code = R"cpp(
+namespace ns { struct S{}; }
+typedef ns::S X;
+[[X]] x;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
+   {"struct S", Rel::Underlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.



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


[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I'm late to the party, but have looked at the code and I really, really-really 
like what you did in this patch! It solves one of my biggest complaints about 
this project. LGTM!




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1389
   HelpText<"Check the analyzer's understanding of C++ iterators">,
-  Dependencies<[IteratorModeling]>,
+  Dependencies<[DebugContainerModeling, IteratorModeling]>,
   Documentation;

This just works??? I admit that I didn't test very thoroughly whether multiple 
dependencies would work out, but I'm glad it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73547



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


[PATCH] D74025: [clangd] Add the missing elaborated types in FindTarget.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeaf0c89ec5f8: [clangd] Add the missing elaborated types in 
FindTarget. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D74025?vs=242524&id=242566#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74025

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -241,6 +241,13 @@
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
+  Code = R"cpp(
+namespace ns { struct S{}; }
+typedef ns::S X;
+[[X]] x;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
+   {"struct S", Rel::Underlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -379,6 +379,10 @@
 Outer.add(TT->getAsTagDecl(), Flags);
   }
 
+  void VisitElaboratedType(const ElaboratedType *ET) {
+Outer.add(ET->desugar(), Flags);
+  }
+
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT) {
 Outer.add(ICNT->getDecl(), Flags);
   }


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -241,6 +241,13 @@
   )cpp";
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
+  Code = R"cpp(
+namespace ns { struct S{}; }
+typedef ns::S X;
+[[X]] x;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc", {"typedef ns::S X", Rel::Alias},
+   {"struct S", Rel::Underlying});
 
   // FIXME: Auto-completion in a template requires disabling delayed template
   // parsing.
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -379,6 +379,10 @@
 Outer.add(TT->getAsTagDecl(), Flags);
   }
 
+  void VisitElaboratedType(const ElaboratedType *ET) {
+Outer.add(ET->desugar(), Flags);
+  }
+
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT) {
 Outer.add(ICNT->getDecl(), Flags);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73651: [OpenCL][CUDA][HIP][SYCL] Add norecurse

2020-02-05 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.

LGTM! Thanks!


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

https://reviews.llvm.org/D73651



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


[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-02-05 Thread Alexey Romanov via Phabricator via cfe-commits
alexeyr added a comment.

In D73775#1856869 , @aaron.ballman 
wrote:

> In D73775#1856848 , @alexeyr wrote:
>
> > In D73775#1856765 , @aaron.ballman 
> > wrote:
> >
> > > In D73775#1851553 , @alexeyr 
> > > wrote:
> > >
> > > > Also I am not sure why, but the ranges added to the diagnostic in lines 
> > > > 1222-1226 don't show up in the message.
> > >
> > >
> > > Do you mean that there are no squiggly underlines under the range, or 
> > > something else? Passing in a range to the diagnostics engine gives it 
> > > something to put an underline under, as in what's under the -12 in: 
> > > https://godbolt.org/z/GeQzYg
> >
> >
> > Yes, exactly. I expect to see the underlines, but don't; only the `^` in 
> > the location provided to `diag` call.
>
>
> The only time I've seen that happen is when the range is invalid. I'm 
> guessing you'll have to step through the diagnostics code to see what's going 
> on.


After stepping through diagnostics code, I... don't understand how it is 
supposed to work. `ClangTidyDiagnosticConsumer` is

> A diagnostic consumer that turns each Diagnostic into a 
> SourceManager-independent ClangTidyError.

And it adds the //hints// to the `ClangTidyError` here 
:

  for (const auto &FixIt : Hints) { ... }

But not the ranges, and I don't see any place where it does or even how they 
can be inserted into a `ClangTidyError` in the first place!


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

https://reviews.llvm.org/D73775



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


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D72705#1838324 , @balazske wrote:

> I am still unsure about how this checker works if the function to check is 
> "modeled" or evaluated by another checker. Then the function may have already 
> a constrained value at the PostCall event (for example if a `malloc` fails) 
> and the basic idea of this checker that the constraints on the return value 
> indicate the checks that the program makes does not work. So it may be better 
> to check for the initial constraint somehow and if found ignore that 
> function. Or create a new state where the function call's value is not 
> constrained.


Under evaluation, you mean `evalCall()`? Could you please detail what your 
worry is, and what the essence of the current solution is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705



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


[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-02-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D73775#1859156 , @alexeyr wrote:

> In D73775#1856869 , @aaron.ballman 
> wrote:
>
> > In D73775#1856848 , @alexeyr wrote:
> >
> > > In D73775#1856765 , 
> > > @aaron.ballman wrote:
> > >
> > > > In D73775#1851553 , @alexeyr 
> > > > wrote:
> > > >
> > > > > Also I am not sure why, but the ranges added to the diagnostic in 
> > > > > lines 1222-1226 don't show up in the message.
> > > >
> > > >
> > > > Do you mean that there are no squiggly underlines under the range, or 
> > > > something else? Passing in a range to the diagnostics engine gives it 
> > > > something to put an underline under, as in what's under the -12 in: 
> > > > https://godbolt.org/z/GeQzYg
> > >
> > >
> > > Yes, exactly. I expect to see the underlines, but don't; only the `^` in 
> > > the location provided to `diag` call.
> >
> >
> > The only time I've seen that happen is when the range is invalid. I'm 
> > guessing you'll have to step through the diagnostics code to see what's 
> > going on.
>
>
> After stepping through diagnostics code, I... don't understand how it is 
> supposed to work. `ClangTidyDiagnosticConsumer` is
>
> > A diagnostic consumer that turns each Diagnostic into a 
> > SourceManager-independent ClangTidyError.
>
> And it adds the //hints// to the `ClangTidyError` here 
> :
>
>   for (const auto &FixIt : Hints) { ... }
>
>
> But not the ranges, and I don't see any place where it does or even how they 
> can be inserted into a `ClangTidyError` in the first place!


Huh, this does seem like it may be a bug in clang-tidy. I would have expected 
`ClangTidyDiagnosticConsumer::forwardDiagnostic()` to do this work.


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

https://reviews.llvm.org/D73775



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


[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-02-05 Thread Alexey Romanov via Phabricator via cfe-commits
alexeyr updated this revision to Diff 242572.
alexeyr added a comment.

Lower-case warning messages


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

https://reviews.llvm.org/D73775

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -81,6 +81,13 @@
   if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both sides of operator are equivalent
 
+  if (X && Y && X) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: operator has equivalent nested operands
+  if (X || (Y || X)) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: operator has equivalent nested operands
+  if ((X ^ Y) ^ (Y ^ X)) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: operator has equivalent nested operands
+
   return 0;
 }
 
@@ -106,6 +113,7 @@
   if (++X != ++X) return 1;
   if (P.a[X]++ != P.a[X]++) return 1;
   if (P.a[X++] != P.a[X++]) return 1;
+  if (X && X++ && X) return 1;
 
   if ("abc" == "ABC") return 1;
   if (foo(bar(0)) < (foo(bat(0, 1 return 1;
@@ -163,6 +171,15 @@
 bool operator>(const MyStruct& lhs, MyStruct& rhs) { rhs.x--; return lhs.x > rhs.x; }
 bool operator||(MyStruct& lhs, const MyStruct& rhs) { lhs.x++; return lhs.x || rhs.x; }
 
+struct MyStruct1 {
+  bool x;
+  MyStruct1(bool x) : x(x) {};
+  operator bool() { return x; }
+};
+
+MyStruct1 operator&&(const MyStruct1& lhs, const MyStruct1& rhs) { return lhs.x && rhs.x; }
+MyStruct1 operator||(MyStruct1& lhs, MyStruct1& rhs) { return lhs.x && rhs.x; }
+
 bool TestOverloadedOperator(MyStruct& S) {
   if (S == Q) return false;
 
@@ -180,6 +197,15 @@
   if (S >= S) return true;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
 
+  MyStruct1 U(false);
+  MyStruct1 V(true);
+
+  // valid because the operator is not const
+  if ((U || V) || U) return true;
+
+  if (U && V && U && V) return true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: overloaded operator has equivalent nested operands
+
   return true;
 }
 
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -18,7 +18,9 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include 
 #include 
 #include 
@@ -304,6 +306,133 @@
   }
 }
 
+// to use in the template below
+static OverloadedOperatorKind getOp(const BinaryOperator *Op) {
+  return BinaryOperator::getOverloadedOperator(Op->getOpcode());
+}
+
+static OverloadedOperatorKind getOp(const CXXOperatorCallExpr *Op) {
+  if (Op->getNumArgs() != 2)
+return OO_None;
+  return Op->getOperator();
+}
+
+static std::pair
+getOperands(const BinaryOperator *Op) {
+  return {Op->getLHS()->IgnoreParenImpCasts(),
+  Op->getRHS()->IgnoreParenImpCasts()};
+}
+
+static std::pair
+getOperands(const CXXOperatorCallExpr *Op) {
+  return {Op->getArg(0)->IgnoreParenImpCasts(),
+  Op->getArg(1)->IgnoreParenImpCasts()};
+}
+
+template 
+static const TExpr *checkOpKind(const Expr *TheExpr,
+OverloadedOperatorKind OpKind) {
+  const auto *AsTExpr = dyn_cast_or_null(TheExpr);
+  if (AsTExpr && getOp(AsTExpr) == OpKind)
+return AsTExpr;
+
+  return nullptr;
+}
+
+// returns true if a subexpression has two directly equivalent operands and
+// is already handled by operands/parametersAreEquivalent
+template 
+static bool collectOperands(const Expr *Part,
+SmallVector &AllOperands,
+OverloadedOperatorKind OpKind) {
+  const auto *PartAsBinOp = checkOpKind(Part, OpKind);
+  if (PartAsBinOp) {
+auto Operands = getOperands(PartAsBinOp);
+if (areEquivalentExpr(Operands.first, Operands.second))
+  return true;
+return collectOperands(Operands.first, AllOperands, OpKind) ||
+   collectOperands(Operands.second, AllOperands, OpKind);
+  } else {
+AllOperands.push_back(Part);
+  }
+  return false;
+}
+
+template 
+static bool hasSameOperatorParent(const Expr *TheExpr,
+  OverloadedOperatorKind OpKind,
+  ASTContext &Context) {
+  // IgnoreParenImpCasts logic in reverse: skip surrounding uninteresting nodes
+  const DynTypedNodeList Parents = Context.getParents(*TheExpr);
+  f

[PATCH] D74044: [ARM] Add initial support for Custom Datapath Extension (CDE)

2020-02-05 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki created this revision.
miyuki added reviewers: simon_tatham, ostannard, dmgreen, eli.friedman.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls.
Herald added projects: clang, LLVM.

This patch adds assembly-level support for a new Arm M-profile
architecture extension, Custom Datapath Extension (CDE).

A brief description of the extension is available at
https://developer.arm.com/architectures/instruction-sets/custom-instructions

The latest specification for CDE is currently a beta release and is
available at
https://static.docs.arm.com/ddi0607/aa/DDI0607A_a_armv8m_arm_supplement_cde.pdf

CDE allows chip vendors to add custom CPU instructions.  The CDE
instructions re-use the same encoding space as existing coprocessor
instructions (such as MRC, MCR, CDP etc.). Each coprocessor in range
cp0-cp7 can be configured as either general purpose (GCP) or custom
datapath (CDEv1).  This configuration is defined by the CPU vendor and
is provided to LLVM using 8 subtarget features: cdecp0 ... cdecp7.

The semantics of CDE instructions are implementation-defined, but the
instructions are guaranteed to be pure (that is, they are stateless,
they do not access memory or any registers except their explicit
inputs/outputs).

CDE requires the CPU to support at least Armv8.0-M mainline
architecture. CDE includes 3 sets of instructions:

- Instructions that operate on general purpose registers and NZCV flags
- Instructions that operate on the S or D register file (require either FP or 
MVE extension)
- Instructions that operate on the Q register file, require MVE

The user-facing names that can be specified on the command line are
the same as the 8 subtarget feature names. For example:

  $ clang -target arm-none-none-eabi -march=armv8m.main+cdecp0+cdecp3

tells the compiler that the coprocessors 0 and 3 are configured as
CDEv1 and the remaining coprocessors are configured as GCP (which is
the default).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74044

Files:
  clang/test/Driver/arm-cde.c
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMInstrCDE.td
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMPredicates.td
  llvm/lib/Target/ARM/ARMRegisterInfo.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.h
  llvm/test/MC/ARM/cde-fp-vec.s
  llvm/test/MC/ARM/cde-integer.s
  llvm/test/MC/Disassembler/ARM/cde-fp-vec.txt
  llvm/test/MC/Disassembler/ARM/cde-integer.txt

Index: llvm/test/MC/Disassembler/ARM/cde-integer.txt
===
--- /dev/null
+++ llvm/test/MC/Disassembler/ARM/cde-integer.txt
@@ -0,0 +1,117 @@
+# RUN: not llvm-mc -disassemble -triple=thumbv8m.main -mattr=+cdecp0 -mattr=+cdecp1 < %s 2>%t | FileCheck %s
+# RUN: FileCheck <%t --check-prefix=ERROR %s
+
+# GCP instructions
+
+# CHECK: mrc p3, #1, r3, c15, c15, #5
+[0x3f,0xee,0xbf,0x33]
+# CHECK-NEXT: mcr2 p3, #2, r2, c7, c11, #7
+[0x47,0xfe,0xfb,0x23]
+
+# CX1
+
+# CHECK-NEXT: cx1	p0, r3, #8191
+[0x3f,0xee,0xbf,0x30]
+# CHECK-NEXT: cx1a	p1, r2, #0
+[0x00,0xfe,0x00,0x21]
+# CHECK-NEXT: cx1d p0, r4, r5, #1234
+[0x09,0xee,0xd2,0x40]
+# CHECK-NEXT: cx1da	p1, r2, r3, #1234
+[0x09,0xfe,0xd2,0x21]
+# CHECK-NEXT: cx1	p0, apsr_nzcv, #8191
+[0x3f,0xee,0xbf,0xf0]
+
+# ERROR: [[@LINE+2]]:{{[0-9]+}}: warning: potentially undefined instruction encoding
+# CHECK-NEXT: cx1 p0, sp, #8191
+[0x3f,0xee,0xbf,0xd0]
+# ERROR: [[@LINE+2]]:{{[0-9]+}}: warning: potentially undefined instruction encoding
+# CHECK-NEXT: cx1d p0, r12, sp, #1234
+[0x09,0xee,0xd2,0xc0]
+# ERROR: [[@LINE+2]]:{{[0-9]+}}: warning: potentially undefined instruction encoding
+# CHECK-NEXT: cx1d p0, r2, r3, #1234
+[0x09,0xee,0xd2,0x30]
+
+# CX2
+
+# CHECK-NEXT: cx2	p0, r3, r7, #0
+[0x47,0xee,0x00,0x30]
+# CHECK-NEXT: cx2a	p0, r1, r4, #511
+[0x74,0xfe,0xbf,0x10]
+# CHECK-NEXT: cx2d	p0, r2, r3, r1, #123
+[0x41,0xee,0xfb,0x20]
+# CHECK-NEXT: cx2da p0, r2, r3, r7, #123
+[0x47,0xfe,0xfb,0x20]
+# CHECK-NEXT: cx2da p1, r10, r11, apsr_nzcv, #123
+[0x4f,0xfe,0xfb,0xa1]
+
+# ERROR: [[@LINE+2]]:{{[0-9]+}}: warning: potentially undefined instruction encoding
+# CHECK-NEXT: cx2a	p0, r1, sp, #511
+[0x7d,0xfe,0xbf,0x10]
+# ERROR: [[@LINE+1]]:{{[0-9]+}}: warning: invalid instruction encoding
+[0x4f,0xfe,0xfb,0xe1]
+
+# CX3
+
+# CHECK-NEXT: cx3 p0, r1, r2, r3, #0
+[0x82,0xee,0x01,0x30]
+# CHECK-NEXT: cx3a p0, r1, r5, r7, #63
+[0xf5,0xfe,0xb1,0x70]
+# CHECK-NEXT: cx3d p1, r0, r1, r7, r1, #12
+[0x97,0xee,0xc0,0x11]
+# CHECK-NEXT: cx3da p0, r8, r9, r2, r3, #12
+[0x92,0xfe,0xc8,0x30]
+# CHECK-NEXT: cx3	p1, apsr_nzcv, r7, apsr_nzcv, #12
+[0x97,0xee,0x8f,0xf1]
+# CHECK-NEXT: cx3d	p0, r8, r9, apsr_nzcv, apsr_nzcv, #12
+[0x9f,0

[PATCH] D73636: [AArch64][SVE] SVE2 intrinsics for complex integer arithmetic

2020-02-05 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1758
+def int_aarch64_sve_sqcadd_x : AdvSIMD_2VectorArgIndexed_Intrinsic;
+def int_aarch64_sve_cmla_x   : AdvSIMD_3VectorArgIndexed_Intrinsic;
+def int_aarch64_sve_cmla_lane_x  : AdvSIMD_SVE_CMLA_LANE_Intrinsic;

While the shape of AdvSIMD_3VectorArgIndexed_Intrinsic may be the same, the 
immediate of int_aarch64_sve_cmla_x is not an index. Instead, it is an 
immediate that expresses the complex rotation. I would just create a separate 
Intrinsic class for that.
Same for `cadd`, `sqcadd` and `sqrdcmlah`


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

https://reviews.llvm.org/D73636



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


[PATCH] D74020: [ARM] Clean up ARM target & feature checking in clang driver.

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74020



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


[PATCH] D73719: [AArch64][SVE] Add SVE2 intrinsics for widening DSP operations

2020-02-05 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73719



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


[PATCH] D73903: [AArch64][SVE] Add remaining SVE2 intrinsics for widening DSP operations

2020-02-05 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1852
 
-// SVE2 MLA LANE.
-def int_aarch64_sve_smlalb_lane   : SVE2_3VectorArg_Indexed_Intrinsic;

nit: why are you moving these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73903



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


[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 242577.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Address the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746

Files:
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolLocation.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  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
@@ -256,21 +256,27 @@
 
 llvm::ArrayRef
 syntax::spelledTokensTouching(SourceLocation Loc,
-  const syntax::TokenBuffer &Tokens) {
+  llvm::ArrayRef Tokens) {
   assert(Loc.isFileID());
-  llvm::ArrayRef All =
-  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
   auto *Right = llvm::partition_point(
-  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
-  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
-  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  Tokens, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != Tokens.end() && Right->location() <= Loc;
+  bool AcceptLeft =
+  Right != Tokens.begin() && (Right - 1)->endLocation() >= Loc;
   return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
 Right + (AcceptRight ? 1 : 0));
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  return spelledTokensTouching(
+  Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 const syntax::Token *
 syntax::spelledIdentifierTouching(SourceLocation Loc,
-  const syntax::TokenBuffer &Tokens) {
+  llvm::ArrayRef Tokens) {
   for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
 if (Tok.kind() == tok::identifier)
   return &Tok;
@@ -278,6 +284,13 @@
   return nullptr;
 }
 
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  return spelledIdentifierTouching(
+  Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -316,11 +316,16 @@
 /// The spelled tokens that overlap or touch a spelling location Loc.
 /// This always returns 0-2 tokens.
 llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, llvm::ArrayRef Tokens);
+llvm::ArrayRef
 spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
 
 /// The identifier token that overlaps or touches a spelling location Loc.
 /// If there is none, returns nullptr.
 const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  llvm::ArrayRef Tokens);
+const syntax::Token *
 spelledIdentifierTouching(SourceLocation Loc,
   const syntax::TokenBuffer &Tokens);
 
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -700,6 +700,40 @@
   EXPECT_THAT(Refs, IsEmpty());
 }
 
+TEST_F(SymbolCollectorTest, SpelledReference) {
+  Annotations Header(R"cpp(
+  struct Foo;
+  #define MACRO Foo
+  )cpp");
+  Annotations Main(R"cpp(
+  struct $spelled[[Foo]] {
+$spelled[[Foo]]();
+~$spelled[[Foo]]();
+  };
+  $spelled[[Foo]] Variable1;
+  $implicit[[MACRO]] Variable2;
+  )cpp");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = false;
+  runSymbolCollector(Header.code(), Main.code());
+  const auto SpelledRanges = Main.ranges("spelled");
+  const auto ImplicitRanges = Main.ranges("implicit");
+  auto AllRanges = SpelledRanges;
+  AllRanges.insert(end(AllRanges), begin(ImplicitRanges), end(ImplicitRanges));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(AllRanges;
+  RefSlab::Builder SpelledSlabBuilder;
+  for (const auto &SymbolAndRefs : Refs) {
+const auto Symbol = SymbolAndRefs.first;
+for (const auto &Ref : SymbolAndRefs.second)
+  if ((Ref.Ki

[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

Looks great, thanks!

Do you have commit access? If so, go ahead and push directly to the 10.x 
branch, otherwise let me know and I'll do it for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031



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


[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 242578.
kbobyrev added a comment.

Remove an artifact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746

Files:
  clang-tools-extra/clangd/index/Ref.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  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
@@ -256,21 +256,27 @@
 
 llvm::ArrayRef
 syntax::spelledTokensTouching(SourceLocation Loc,
-  const syntax::TokenBuffer &Tokens) {
+  llvm::ArrayRef Tokens) {
   assert(Loc.isFileID());
-  llvm::ArrayRef All =
-  Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
   auto *Right = llvm::partition_point(
-  All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
-  bool AcceptRight = Right != All.end() && Right->location() <= Loc;
-  bool AcceptLeft = Right != All.begin() && (Right - 1)->endLocation() >= Loc;
+  Tokens, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
+  bool AcceptRight = Right != Tokens.end() && Right->location() <= Loc;
+  bool AcceptLeft =
+  Right != Tokens.begin() && (Right - 1)->endLocation() >= Loc;
   return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
 Right + (AcceptRight ? 1 : 0));
 }
 
+llvm::ArrayRef
+syntax::spelledTokensTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  return spelledTokensTouching(
+  Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 const syntax::Token *
 syntax::spelledIdentifierTouching(SourceLocation Loc,
-  const syntax::TokenBuffer &Tokens) {
+  llvm::ArrayRef Tokens) {
   for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) {
 if (Tok.kind() == tok::identifier)
   return &Tok;
@@ -278,6 +284,13 @@
   return nullptr;
 }
 
+const syntax::Token *
+syntax::spelledIdentifierTouching(SourceLocation Loc,
+  const syntax::TokenBuffer &Tokens) {
+  return spelledIdentifierTouching(
+  Loc, Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc)));
+}
+
 std::vector
 TokenBuffer::macroExpansions(FileID FID) const {
   auto FileIt = Files.find(FID);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -316,11 +316,16 @@
 /// The spelled tokens that overlap or touch a spelling location Loc.
 /// This always returns 0-2 tokens.
 llvm::ArrayRef
+spelledTokensTouching(SourceLocation Loc, llvm::ArrayRef Tokens);
+llvm::ArrayRef
 spelledTokensTouching(SourceLocation Loc, const syntax::TokenBuffer &Tokens);
 
 /// The identifier token that overlaps or touches a spelling location Loc.
 /// If there is none, returns nullptr.
 const syntax::Token *
+spelledIdentifierTouching(SourceLocation Loc,
+  llvm::ArrayRef Tokens);
+const syntax::Token *
 spelledIdentifierTouching(SourceLocation Loc,
   const syntax::TokenBuffer &Tokens);
 
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -700,6 +700,40 @@
   EXPECT_THAT(Refs, IsEmpty());
 }
 
+TEST_F(SymbolCollectorTest, SpelledReference) {
+  Annotations Header(R"cpp(
+  struct Foo;
+  #define MACRO Foo
+  )cpp");
+  Annotations Main(R"cpp(
+  struct $spelled[[Foo]] {
+$spelled[[Foo]]();
+~$spelled[[Foo]]();
+  };
+  $spelled[[Foo]] Variable1;
+  $implicit[[MACRO]] Variable2;
+  )cpp");
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = false;
+  runSymbolCollector(Header.code(), Main.code());
+  const auto SpelledRanges = Main.ranges("spelled");
+  const auto ImplicitRanges = Main.ranges("implicit");
+  auto AllRanges = SpelledRanges;
+  AllRanges.insert(end(AllRanges), begin(ImplicitRanges), end(ImplicitRanges));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(AllRanges;
+  RefSlab::Builder SpelledSlabBuilder;
+  for (const auto &SymbolAndRefs : Refs) {
+const auto Symbol = SymbolAndRefs.first;
+for (const auto &Ref : SymbolAndRefs.second)
+  if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
+SpelledSlabBuilder.insert(Symbol, Ref);
+ 

[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

2020-02-05 Thread Alexey Romanov via Phabricator via cfe-commits
alexeyr added a comment.

In D73775#1859167 , @aaron.ballman 
wrote:

> Huh, this does seem like it may be a bug in clang-tidy. I would have expected 
> `ClangTidyDiagnosticConsumer::forwardDiagnostic()` to do this work.


From my debugging attempt it seems not to be called (it's only supposed to be 
"If there is an external diagnostics engine, like in the ClangTidyPluginAction 
case"). Reported at https://bugs.llvm.org/show_bug.cgi?id=44799.


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

https://reviews.llvm.org/D73775



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


[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:644
   Dependencies<[ContainerModeling]>,
-  Documentation,
-  Hidden;
-
-def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
-  HelpText<"Check for use of invalidated iterators">,
-  Dependencies<[IteratorModeling]>,
-  Documentation;
-
-def IteratorRangeChecker : Checker<"IteratorRange">,
-  HelpText<"Check for iterators used outside their valid ranges">,
-  Dependencies<[IteratorModeling]>,
-  Documentation;
-
-def MismatchedIteratorChecker : Checker<"MismatchedIterator">,
-  HelpText<"Check for use of iterators of different containers where iterators 
"
-   "of the same container are expected">,
-  Dependencies<[IteratorModeling]>,
-  Documentation;
-
-} // end: "alpha.cplusplus"
+  Documentation;
 

This checker should be `Hidden`.



Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:37-38
+
+  typedef bool (STLAlgorithmModeling::*FnCheck)(CheckerContext &,
+const CallExpr *) const;
+

Prefer `using`. D67706#inline-614013



Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:67
+public:
+  STLAlgorithmModeling() {}
+

` = default;`



Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:126
+  // assume that in case of successful search the position of the found element
+  // is ahed of it.
+  const auto *Pos = getIteratorPosition(State, SecondParam);

NoQ wrote:
> Typo: *ahead.
Hold on, isn't it the other way around?

```
[_|_|x|_|_|_|_|y|_|_|_|z|_]
```
Suppose in the range `[x, z)` I'm looking for `y`. The range-end argument would 
be `z`, isn't `y` behind it? The following code and the test cases seem to be 
correct, so I guess its the comment?



Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:129-130
+  if (Pos) {
+StateFound = createIteratorPosition(StateFound, RetVal, 
Pos->getContainer(),
+CE, LCtx, C.blockCount());
+const auto *NewPos = getIteratorPosition(StateFound, RetVal);

What if the range-end iterator is a reverse iterator? Wouldn't it mess with the 
relative position of the found element?

```
[_|_|x|_|_|_|_|y|_|_|_|z|_]

std::find(z, x, y);
```

Suppose I'm searching in the `(x, z]` range for `y`, where `x`,  `z` are 
reverse iterators. Here, you'll create a forward iterator for `y`, if I'm not 
mistaken, and you'd say that its behind `x`? Are these things correctly modeled 
in IteratorChecker?


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

https://reviews.llvm.org/D70818



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


[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-05 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a subscriber: martong.
steakhal added a comment.

I'm convinced that we shouldn't remove taint from expressions used in 
comparisons.

With the current configuration files, `sink` functions are not too useful.
For now, I would delay developing a mechanism describing constraints here, 
since @martong is working on function summaries in D73897 
,D73898 .
In function summaries we could describe how should a given function react to a 
tainted parameter. Which would draw `sink` functions in the taint config file 
meaningless.

I'm planning to abandon this patch if you don't have any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73536



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Andi via Phabricator via cfe-commits
Abpostelnicu added a comment.

In D74031#1859231 , @hans wrote:

> Looks great, thanks!
>
> Do you have commit access? If so, go ahead and push directly to the 10.x 
> branch, otherwise let me know and I'll do it for you.


Yes I have commit access, pushing it directly to `10.x` then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031



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


[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746



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


[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Also, I really like this patch, its well documented, small and very well tested!


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

https://reviews.llvm.org/D70818



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


[PATCH] D74046: [clang][driver] Fix null pointer dereference warning inside PrintActions1 (PR43462)

2020-02-05 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon created this revision.
RKSimon added reviewers: sfantao, tra, ABataev, jlebar.
Herald added a project: clang.

As detailed on PR43462, clang static analyzer is complaining about a null 
pointer dereference as we provide a 'host' toolchain fallback if the ToolChain 
pointer is null, but then use that pointer anyhow to report the triple.

Tests indicate the ToolChain pointer is always valid and the 'host' code path 
is redundant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74046

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1848,6 +1848,7 @@
 bool IsFirst = true;
 OA->doOnEachDependence(
 [&](Action *A, const ToolChain *TC, const char *BoundArch) {
+  assert(TC && "Unknown host toolchain");
   // E.g. for two CUDA device dependences whose bound arch is sm_20 and
   // sm_35 this will generate:
   // "cuda-device" (nvptx64-nvidia-cuda:sm_20) {#ID}, "cuda-device"
@@ -1855,13 +1856,9 @@
   if (!IsFirst)
 os << ", ";
   os << '"';
-  if (TC)
-os << A->getOffloadingKindPrefix();
-  else
-os << "host";
+  os << A->getOffloadingKindPrefix();
   os << " (";
   os << TC->getTriple().normalize();
-
   if (BoundArch)
 os << ":" << BoundArch;
   os << ")";


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1848,6 +1848,7 @@
 bool IsFirst = true;
 OA->doOnEachDependence(
 [&](Action *A, const ToolChain *TC, const char *BoundArch) {
+  assert(TC && "Unknown host toolchain");
   // E.g. for two CUDA device dependences whose bound arch is sm_20 and
   // sm_35 this will generate:
   // "cuda-device" (nvptx64-nvidia-cuda:sm_20) {#ID}, "cuda-device"
@@ -1855,13 +1856,9 @@
   if (!IsFirst)
 os << ", ";
   os << '"';
-  if (TC)
-os << A->getOffloadingKindPrefix();
-  else
-os << "host";
+  os << A->getOffloadingKindPrefix();
   os << " (";
   os << TC->getTriple().normalize();
-
   if (BoundArch)
 os << ":" << BoundArch;
   os << ")";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I think its very good that this conversation came up, and it might just happen 
that we'll end up removing some taint when we have a better understanding of 
how this works. For now, I think we can put this aside :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73536



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


[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

2020-02-05 Thread Andi via Phabricator via cfe-commits
Abpostelnicu added a comment.

done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74031



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


[clang] 482e236 - [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-05 Thread Artem Dergachev via cfe-commits

Author: Artem Dergachev
Date: 2020-02-05T17:16:38+03:00
New Revision: 482e236e569e8324f70778af1eb756923cd490dc

URL: 
https://github.com/llvm/llvm-project/commit/482e236e569e8324f70778af1eb756923cd490dc
DIFF: 
https://github.com/llvm/llvm-project/commit/482e236e569e8324f70778af1eb756923cd490dc.diff

LOG: [analyzer] Fix a couple of bugs in HTML report generation.

It should now produce valid HTML again.

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

Added: 
clang/test/Analysis/html_diagnostics/td-hotfix.c
clang/test/Analysis/html_diagnostics/variable-popups-macro.c
clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
clang/test/Analysis/html_diagnostics/variable-popups-simple.c

Modified: 
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index a4918d7179ff..002b6070ddcd 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -607,10 +607,17 @@ window.addEventListener("keydown", function (event) {
 )<<<";
 }
 
+static bool shouldDisplayPopUpRange(const SourceRange &Range) {
+  return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID());
+}
+
 static void
 HandlePopUpPieceStartTag(Rewriter &R,
  const std::vector &PopUpRanges) {
   for (const auto &Range : PopUpRanges) {
+if (!shouldDisplayPopUpRange(Range))
+  continue;
+
 html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "",
  "",
  /*IsTokenRange=*/true);
@@ -626,6 +633,8 @@ static void HandlePopUpPieceEndTag(Rewriter &R,
   llvm::raw_svector_ostream Out(Buf);
 
   SourceRange Range(Piece.getLocation().asRange());
+  if (!shouldDisplayPopUpRange(Range))
+return;
 
   // Write out the path indices with a right arrow and the message as a row.
   Out << ""
@@ -870,7 +879,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter &R, FileID 
BugFileID,
  << (num - 1)
  << "\" title=\"Previous event ("
  << (num - 1)
- << ")\">←";
+ << ")\">←";
 }
 
 os << "";

diff  --git a/clang/test/Analysis/html_diagnostics/td-hotfix.c 
b/clang/test/Analysis/html_diagnostics/td-hotfix.c
new file mode 100644
index ..8595642ad0f5
--- /dev/null
+++ b/clang/test/Analysis/html_diagnostics/td-hotfix.c
@@ -0,0 +1,31 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo() {
+  int a;
+  bar(a); // expected-warning{{1st function call argument is an uninitialized 
value}}
+}
+
+// CHECK-LABEL:
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 2
+// CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   ←
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-NOT:
+// CHECK-SAME:   
+// CHECK-SAME: 1st function call argument is an uninitialized value
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 

diff  --git a/clang/test/Analysis/html_diagnostics/variable-popups-macro.c 
b/clang/test/Analysis/html_diagnostics/variable-popups-macro.c
new file mode 100644
index ..83bda14d4f2f
--- /dev/null
+++ b/clang/test/Analysis/html_diagnostics/variable-popups-macro.c
@@ -0,0 +1,28 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+#define MACRO if (b)
+
+void foo2() {
+  int a;
+  int b = 1;
+  MACRO
+bar(a); // expected-warning{{1st function call argument is an 
uninitialized value}}
+}
+
+// For now we don't emit popups inside macros due to UI limitations.
+// Once we do, we should test it thoroughly.
+
+// CHECK-LABEL: 
+// CHECK-NOT:   
+// CHECK-SAME:  
+// CHECK-SAME:MACRO
+// CHECK-SAME:
+// CHECK-SAME:  if (b)
+// CHECK-SAME:
+// CHECK-SAME:  

diff  --git a/clang/test/Analysis/html_diagnostics/variable-popups-multiple.c 
b/clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
new file mode 100644
index ..d7a05b53e4f5
--- /dev/null
+++ b/clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
@@ -0,0 +1,29 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo() {
+  int a;
+  for (unsigned i = 0; i < 3; ++i)
+if (i)
+  bar(a)

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-05 Thread Artem Dergachev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG482e236e569e: [analyzer] Fix a couple of bugs in HTML report 
generation. (authored by dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73993

Files:
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/html_diagnostics/td-hotfix.c
  clang/test/Analysis/html_diagnostics/variable-popups-macro.c
  clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
  clang/test/Analysis/html_diagnostics/variable-popups-simple.c

Index: clang/test/Analysis/html_diagnostics/variable-popups-simple.c
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/variable-popups-simple.c
@@ -0,0 +1,23 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo2() {
+  int a;
+  int b = 1;
+  if (b)
+bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// CHECK:  b
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   1.1
+// CHECK-SAME: 
+// CHECK-SAME: 'b' is 1
+// CHECK-SAME:   
+// CHECK-SAME: 
Index: clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/variable-popups-multiple.c
@@ -0,0 +1,29 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo() {
+  int a;
+  for (unsigned i = 0; i < 3; ++i)
+if (i)
+  bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// CHECK:  i
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   2.1
+// CHECK-SAME: 
+// CHECK-SAME: 'i' is 0
+// CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   4.1
+// CHECK-SAME: 
+// CHECK-SAME: 'i' is 1
+// CHECK-SAME:   
+// CHECK-SAME: 
Index: clang/test/Analysis/html_diagnostics/variable-popups-macro.c
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/variable-popups-macro.c
@@ -0,0 +1,28 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+#define MACRO if (b)
+
+void foo2() {
+  int a;
+  int b = 1;
+  MACRO
+bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// For now we don't emit popups inside macros due to UI limitations.
+// Once we do, we should test it thoroughly.
+
+// CHECK-LABEL: 
+// CHECK-NOT:   
+// CHECK-SAME:  
+// CHECK-SAME:MACRO
+// CHECK-SAME:
+// CHECK-SAME:  if (b)
+// CHECK-SAME:
+// CHECK-SAME:  
Index: clang/test/Analysis/html_diagnostics/td-hotfix.c
===
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/td-hotfix.c
@@ -0,0 +1,31 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:-analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo() {
+  int a;
+  bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// CHECK-LABEL:
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 2
+// CHECK-SAME:   
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   ←
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-NOT:
+// CHECK-SAME:   
+// CHECK-SAME: 1st function call argument is an uninitialized value
+// CHECK-SAME:   
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME: 
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -607,10 +607,17 @@
 )<<<";
 }
 
+static bool shouldDisplayPopUpRange(const SourceRange &Range) {
+  return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID());
+}
+
 static void
 HandlePopUpPieceStartTag(Rewriter &R,
  const std::vector &PopUpRanges) {
   for (const auto &Range : PopUpRanges) {
+if (!shouldDisplayPopUpRange(Range))
+  continue;
+
 html::HighlightRange(R, Range.getBegin(), Range.get

[PATCH] D69582: Let clang driver support parallel jobs

2020-02-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: clang/lib/Driver/Compilation.cpp:332
+if (!Next) {
+  std::this_thread::yield();
   continue;

yaxunl wrote:
> aganea wrote:
> > In addition to what @thakis said above, yielding here is maybe not a good 
> > idea. This causes the process to spin, and remain in the OS' active process 
> > list, which uselessly eats cpu cycles. This can become significant over the 
> > course of several minutes of compilation.
> > 
> > Here's a //tiny// example of what happens when threads are waiting for 
> > something to happen:
> > (the top parts yields frequently; the bottom part does not yield - see 
> > D68820)
> > {F10592208}
> > 
> > You would need here to go through a OS primitive that suspends the process 
> > until at least one job in the pool completes. On Windows this can be 
> > achieved through `WaitForMultipleObjects()` or I/O completion ports like 
> > provided by @thakis. You can take a look at `Compilation::executeJobs()` in 
> > D52193 and further down the line, `WaitMany()` which waits for at least one 
> > job/process to complete.
> Sorry for the delay.
> 
> If D52193 is commited, I will probably only need some minor change to support 
> parallel compilation for HIP. Therefore I hope D52193 could get committed 
> soon.
> 
> I am wondering what is the current status of D52193 and what is blocking it. 
> Is there any chance to get it commited soon?
> 
> Thanks.
Hi @yaxunl! Nothing prevents from finishing D52193 :-) It was meant as a 
prototype, but I could transform it into a more desirable state.
I left it aside because we made another (unpublished) prototype, where the 
process invocations were in fact collapsed into the calling process, ie. ran in 
a thread pool in the manner of the recent `-fintegrated-cc1` flag. But that 
requires for `cl::opt` to support different contexts, as opposed to just one 
global state ([[ 
http://lists.llvm.org/pipermail/llvm-dev/2018-October/127039.html | an RFC was 
discussed ]] about a year ago, but there was no consensus).
Having a thread pool instead of the process pool is faster when compiling 
.C/.CPP files with `clang-cl /MP`, but perhaps in your case that won't work, 
you need to call external binaries, do you? Binaries that are not part of LLVM? 
If so, then landing D52193 first would makes sense.


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

https://reviews.llvm.org/D69582



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


[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread Alexey Bader via Phabricator via cfe-commits
bader created this revision.
bader added a reviewer: ABataev.
Herald added subscribers: cfe-commits, Anastasia, ebevhan.
Herald added a project: clang.

As a first step this implementation enables compilation of the offload
code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74048

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/sycl.c


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4010,6 +4010,11 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP 
device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3405,6 +3408,9 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
 
 include "CC1Options.td"
 


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4010,6 +4010,11 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3405,6 +3408,9 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
 
 include "CC1Options.td"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

2020-02-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

> D73760  will be nice to be cherry picked if 
> there is not much trouble, so we won't have a major release with 
> unsatisfactory label placement if Linux x86 developers ever want to adopt 
> -fpatchable-function-entry=.

Cherry-picked as 4c96b369a074e93a0be536dd795d3f245ef6f18b 
. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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


[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74048



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


[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:612
+  return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID());
+}
+

Charusso wrote:
> Side note: I like the other form of De Morgan's laws because here I have to 
> apply it in my head every time I see such a construct. Also we are using this 
> function in negation, so I would write:
> ```lang=c
> static bool isMacro(const SourceRange &Range) {
>   return Range.getBegin().isMacroID() || Range.getEnd().isMacroID();
> }
> ```
> 
> The idiom is to write code for readability so that understandability over 
> everything else.
Objection :) I want this function to figure out if a pop-up range should be 
displayed, not whether the range is in a macro. Like, if we come up with other 
excuses for not displaying a range, we'll update this function, not make a new 
one. And if we need to check whether something is a macro in another place, 
we'll make a new function, not re-use this one.

I could do something like `shouldSkipPopUpRange()` but that'd move the 
confusing negation into the function name, making call sites harder to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73993



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


[PATCH] D74046: [clang][driver] Fix null pointer dereference warning inside PrintActions1 (PR43462)

2020-02-05 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74046



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


[PATCH] D68923: Don't warn about missing declarations for partial template specializations

2020-02-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D68923#1857307 , @aaronpuchert 
wrote:

> @hans, could you cherry-pick this on the version 10 branch? As I wrote in 
> D68923#1857046 , this is a 
> regression from Clang 8.


Sounds good to me, cherry-picked as fd271fd64a284e9182c8afd8eb8084d8d43df587 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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


[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson marked an inline comment as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+// breaks any uses on assumes.
+if (TypeIdMap.count(TypeId))
+  continue;

evgeny777 wrote:
> tejohnson wrote:
> > evgeny777 wrote:
> > > tejohnson wrote:
> > > > evgeny777 wrote:
> > > > > tejohnson wrote:
> > > > > > evgeny777 wrote:
> > > > > > > I don't think, I understand this.
> > > > > > > It looks like that if (a) we have type.test in the module and (b) 
> > > > > > > we don't have vtable definition with corresponding type metadata 
> > > > > > > in the same module then we'll remove type.test/assume sequence 
> > > > > > > before even getting to ICP. This seems strange in the context of 
> > > > > > > previous discussion because virtual function may be called in 
> > > > > > > different module from one where vtable is defined, like so:
> > > > > > > ```
> > > > > > > struct A { virtual int foo() { return 42; } };
> > > > > > > 
> > > > > > > // calls pA->foo
> > > > > > > extern int callFoo(A *pA);
> > > > > > > int main() { A a; return callFoo(&a); }
> > > > > > > ```
> > > > > > > 
> > > > > > We will only be able to do ICP this way if we have the target 
> > > > > > vtable in the module (similar to how we currently can only do ICP 
> > > > > > if the target function is in the module). I anticipate doing 
> > > > > > something similar for vtable defs as to what we do for virtual 
> > > > > > functions, where a fake call edge is added to the summary based on 
> > > > > > the value profile results to ensure they are imported before the 
> > > > > > LTO backend ICP.
> > > > > > We will only be able to do ICP this way if we have the target 
> > > > > > vtable in the module (similar to how we currently can only do ICP 
> > > > > > if the target function is in the module).
> > > > > 
> > > > > I was thinking of slightly different approach: it seems you have 
> > > > > required type information in combined index together with type name 
> > > > > in the module, so you probably can add external declarations of 
> > > > > required vtables (this may require promotion) to the module in the 
> > > > > ICP pass and run ICP over them. Do you think this can work?
> > > > Possibly, but we'd still need to have type metadata on those vtable 
> > > > declarations, because the type metadata provides the address point 
> > > > offset which is needed in the comparison sequence.
> > > > Possibly, but we'd still need to have type metadata on those vtable 
> > > > declarations, because the type metadata provides the address point 
> > > > offset which is needed in the comparison sequence.
> > > 
> > > I think it's stored in the index in VFuncId structures. Isn't it?
> > > I think it's stored in the index in VFuncId structures. Isn't it?
> > 
> > No, that holds the offset from the address point to the virtual function 
> > within the vtable def, not the address point offset itself, which is what 
> > we need from the type metadata. But actually, we need both offsets:
> > 
> > Consider the following example:
> > 
> > ```
> > class A {
> >virtual void foo();
> > }
> > 
> > void bar(A *a) {
> >a->foo();
> > }
> > ```
> > 
> > The indirect call sequence will look like (not showing the type test for 
> > simplicity):
> > 
> > ```
> >  %0 = bitcast %class.A* %a to i32 (%class.A*)***
> >   %vtable = load i32 (%class.A*)**, i32 (%class.A*)*** %0
> >   %1 = load i32 (%class.A*)*, i32 (%class.A*)** %vtable
> >   %call = tail call i32 %1(%class.A* %a)
> > ```
> > 
> > With type profiling we will profile the value of %vtable, and figure out 
> > that it is associated with the symbol for A's vtable (the profiling support 
> > uses symbol table info for this), which is:
> > 
> > ```
> > @_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* 
> > null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (i32 
> > (%class.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0
> > 
> > !0 = !{i64 16, !"_ZTS1A"}
> > ```
> > 
> > When we promote the call using the vtable profile results, we need to add 
> > the address point offset (16) from the type metadata to the profiled result 
> > which will be the symbol @_ZTV1A, before comparing against %vtable.
> > 
> > We then need to look at the IR and compute the offset to the function 
> > address used there (0 in the above IR), and add it to the address point 
> > offset, resulting in 16 here, looking in the vtable def to see what 
> > function is at the combined offset (_ZN1A3fooEv here), so that we insert a 
> > direct call to that function.
> > 
> > For the address point offset, we only have it in the summary for index-only 
> > WPD (TypeIdCompatibleVtableMap). However, I don't want to restrict the ICP 
> > improvements to that build mode.
> > 
> > For the latter, the VFuncId does summarize the offsets, but we can compute 
> > it from the IR as I described above (it's the amount add

[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3411-3413
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;

Usually, we add a pair of such flags, `-fsycl` and `fno-sycl` to have the 
explicit way to disable some functionality.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4014-4016
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }

No need for braces here



Comment at: clang/test/Driver/sycl.c:5
+
+// DEFAULT: "-fsycl-is-device"

I would add a check that this flag is found on clang invocation with `-cc1` 
option (i.e. it is passed to the frontend)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74048



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


[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread Alexey Bader via Phabricator via cfe-commits
bader updated this revision to Diff 242590.
bader added a comment.

Rebased on ToT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74048

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/sycl.c


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,6 +4023,11 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP 
device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3407,6 +3410,9 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
 
 include "CC1Options.td"
 


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=DEFAULT
+
+// DEFAULT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,6 +4023,11 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = Args.hasArg(options::OPT_fsycl);
+  if (IsSYCL) {
+CmdArgs.push_back("-fsycl-is-device");
+  }
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3407,6 +3410,9 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
 
 include "CC1Options.td"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
console-log.txt 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74048



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


[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added subscribers: steakhal, boga95.
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731
+  }
+  return C.getNoteTag([Text, Name](BugReport &BR) -> std::string {
+  SmallString<256> Msg;

NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > You'll need to check whether the container is actually of interest to the 
> > > bug report. We don't want notes to be added about changes to irrelevant 
> > > containers.
> > > 
> > > You can use a combination of "Report `BR` was emitted by one of the 
> > > iterator checkers" and "The memory region of the container is marked as 
> > > interesting" (while also actually marking it as interesting in the 
> > > checker).
> > > 
> > > Ideally we should instead make a new generic storage inside the 
> > > `BugReport` object, in order to pass down the interesting information 
> > > from the call site of `emitReport` ("Hi, i'm an iterator checker who 
> > > emitted this report and i'm interested in changes made to the size of 
> > > this container").
> > Are you sure in this? I already wondered how it works so I added a test 
> > that checks one container and changes another one and there were no note 
> > tags displayed for the one we did not check but change. See the last test.
> That's because you didn't do
> ```lang=c++
>   V2.cbegin();
>   V2.cend();
> ```
> in the beginning.
A similar conversation sparked up recently in between @boga95, @steakhal and me 
regarding reporting taintedness. Bug reports are fine up to the point where (in 
reverse) the first propagation happens, but finding out which value tainted the 
one that caused the report isn't handled at the moment. One idea was to mark 
the initial (again, in reverse) value as interesting, create a `NoteTag` at the 
point of propagation, where we should know which value was the cause of the 
spread, mark that interesting as well, etc.

If `NoteTag`s only emit a message when the concerning value is interesting, 
this should theoretically solve that problem. I guess you could say that we're 
propagating interestingness in reverse.

I'm not immediately sure if this idea was ever mentioned or implemented here.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:58
 public:
   ContainerModeling() {}
 

While we're at it [part 2], can we make this `= default`? :)



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:68-69
+  SVal) const;
+  typedef void (ContainerModeling::*TwoItParamFn)(CheckerContext &, SVal, SVal,
+  SVal) const;
 

Prefer `using`.


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

https://reviews.llvm.org/D73720



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


[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

2020-02-05 Thread Richard Barton via Phabricator via cfe-commits
richard.barton.arm added inline comments.



Comment at: clang/include/clang/Driver/Options.td:264
   MetaVarName<"">;
+def fcc_fortran_name : Separate<["-"], "ffc-fortran-name">, InternalDriverOpt,
+  HelpText<"Name for native Fortran compiler">;

CarolineConcatto wrote:
> richard.barton.arm wrote:
> > I'm not sure about this name. My memory is not long enough to know what ccc 
> > stands/stood for but git blame suggests it may have been a precursor name 
> > to clang. Although it might seem odd to add new options like this given the 
> > name perhaps doesn't mean anything anymore, I suggest copying this 
> > convention and make this option start with `-ccc`. I also think flang would 
> > be better than fortran here as it better describes what the option is 
> > doing, i.e. telling clang what flang is called.
> > 
> > So recommend `-ccc-flang-name` for the option and commensurate changes to 
> > the internal variables to match.
> I really don't mind changing the name. The way it is stands ffc for: flang fc1
> As I thought that ccc was for clang cc1
> As I thought that ccc was for clang cc1

Hmm - that might be right right. It is certainly as good a guess as any.
I was also thinking maybe it was "Cross CC" or something like that given the 
use of the word "native" in the help text.

Perhaps we should just not use the naming convention as we don't really 
understand it. Maybe `-flang-fc1` or `-fortran-fe` might be a better name?

Anyhow - given what you say, I'll withdraw my quibbling on the name. It is an 
internal name after all. Happy to go with whatever you chose, including the 
original.



Comment at: clang/test/Driver/flang/clang-driver-2-frontend01.f90:14
+! RUN: %clang --driver-mode=flang -ffc-fortran-name %basename_t.tmp1 -### %s 
2>&1 | FileCheck --check-prefixes=ALL,CHECK-EMIT-OBJ %s
+! CHECK-EMIT-OBJ-DAG: "-emit-obj"
+! CHECK-EMIT-OBJ-DAG: "-o" "{{[^"]*}}.o

CarolineConcatto wrote:
> richard.barton.arm wrote:
> > I don't understand why you are checking for this option. Surely the only 
> > relevant check is the "ALL-LABEL" check at the top which checks for the 
> > flang subprocess invocation? Can you explain why you need this check?
> Mainly for sanity check as the patch add a new flag. The logic added does not 
> mess with the previous flags, but I thought it was good to do at least a 
> minimum check.
> 
> If changing I would add more tests like the ones made in flang.f90 and 
> flang.F90 as the one in these files do not use ffc-fortran-name.
I agree with you that the check is correct, -emit-obj will indeed be emitted so 
the check is a correct one for clang's current behaviour.

Where I am coming from is that there are already tests that check that when 
clang is called without `-c` (or `-S`, etc.) that it passes `-emit-obj` to the 
frontend. This test should be checking that `-ffc-fortran-name` causes the 
frontend that clang calls to change. It's not really concerned with the 
`-emit-obj` behaviour. But, because you have this CHECK line in the code, the 
test depends on it to pass. Say we changed the spelling of that option 
-emit-obj or we changed the behaviour of clang in some way that causes 
`-emit-obj` no longer to be passed to the FE in these circumstances. This test 
would still fail if not updated, even though it is supposed to be unrelated to 
the feature being changed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73951



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


[clang] fd3437a - [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

2020-02-05 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-02-05T09:57:53-05:00
New Revision: fd3437a4f791cb0520e19b87953141fc68543377

URL: 
https://github.com/llvm/llvm-project/commit/fd3437a4f791cb0520e19b87953141fc68543377
DIFF: 
https://github.com/llvm/llvm-project/commit/fd3437a4f791cb0520e19b87953141fc68543377.diff

LOG: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

Summary:
To use new/delete in NVPTX code we need to define them. Implementation
copied from CUDA wrappers.

Reviewers: hfinkel, jdoerfert

Subscribers: mgorny, guansong, kkwli0, caomhin, cfe-commits

Tags: #clang

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

Added: 
clang/lib/Headers/openmp_wrappers/new

Modified: 
clang/lib/Headers/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index 85c3124234ad..f172d7a1203f 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -144,6 +144,7 @@ set(openmp_wrapper_files
   openmp_wrappers/cmath
   openmp_wrappers/__clang_openmp_math.h
   openmp_wrappers/__clang_openmp_math_declares.h
+  openmp_wrappers/new
 )
 
 set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)

diff  --git a/clang/lib/Headers/openmp_wrappers/new 
b/clang/lib/Headers/openmp_wrappers/new
new file mode 100644
index ..1387d925b126
--- /dev/null
+++ b/clang/lib/Headers/openmp_wrappers/new
@@ -0,0 +1,70 @@
+//===- new - OPENMP wrapper for  --===
+//
+// 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
+//
+//===---===
+
+#ifndef __CLANG_OPENMP_WRAPPERS_NEW
+#define __CLANG_OPENMP_WRAPPERS_NEW
+
+#include_next 
+
+#if defined(__NVPTX__) && defined(_OPENMP)
+
+#include 
+
+#pragma push_macro("OPENMP_NOEXCEPT")
+#if __cplusplus >= 201103L
+#define OPENMP_NOEXCEPT noexcept
+#else
+#define OPENMP_NOEXCEPT
+#endif
+
+// Device overrides for non-placement new and delete.
+inline void *operator new(__SIZE_TYPE__ size) {
+  if (size == 0)
+size = 1;
+  return ::malloc(size);
+}
+inline void *operator new(__SIZE_TYPE__ size,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  return ::operator new(size);
+}
+
+inline void *operator new[](__SIZE_TYPE__ size) { return ::operator new(size); 
}
+inline void *operator new[](__SIZE_TYPE__ size, const std::nothrow_t &) {
+  return ::operator new(size);
+}
+
+inline void operator delete(void *ptr)OPENMP_NOEXCEPT {
+  if (ptr)
+::free(ptr);
+}
+inline void operator delete(void *ptr, const std::nothrow_t &)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+inline void operator delete[](void *ptr) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline void operator delete[](void *ptr,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+// Sized delete, C++14 only.
+#if __cplusplus >= 201402L
+inline void operator delete(void *ptr, __SIZE_TYPE__ size)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline void operator delete[](void *ptr, __SIZE_TYPE__ size) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+#endif
+
+#pragma pop_macro("OPENMP_NOEXCEPT")
+#endif
+
+#endif // include guard



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


[PATCH] D73128: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

2020-02-05 Thread Alexey Bataev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd3437a4f791: [OPENMP][NVPTX]Add NVPTX specific definitions 
for new/delete operators. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73128

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/openmp_wrappers/new


Index: clang/lib/Headers/openmp_wrappers/new
===
--- /dev/null
+++ clang/lib/Headers/openmp_wrappers/new
@@ -0,0 +1,70 @@
+//===- new - OPENMP wrapper for  --===
+//
+// 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
+//
+//===---===
+
+#ifndef __CLANG_OPENMP_WRAPPERS_NEW
+#define __CLANG_OPENMP_WRAPPERS_NEW
+
+#include_next 
+
+#if defined(__NVPTX__) && defined(_OPENMP)
+
+#include 
+
+#pragma push_macro("OPENMP_NOEXCEPT")
+#if __cplusplus >= 201103L
+#define OPENMP_NOEXCEPT noexcept
+#else
+#define OPENMP_NOEXCEPT
+#endif
+
+// Device overrides for non-placement new and delete.
+inline void *operator new(__SIZE_TYPE__ size) {
+  if (size == 0)
+size = 1;
+  return ::malloc(size);
+}
+inline void *operator new(__SIZE_TYPE__ size,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  return ::operator new(size);
+}
+
+inline void *operator new[](__SIZE_TYPE__ size) { return ::operator new(size); 
}
+inline void *operator new[](__SIZE_TYPE__ size, const std::nothrow_t &) {
+  return ::operator new(size);
+}
+
+inline void operator delete(void *ptr)OPENMP_NOEXCEPT {
+  if (ptr)
+::free(ptr);
+}
+inline void operator delete(void *ptr, const std::nothrow_t &)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+inline void operator delete[](void *ptr) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline void operator delete[](void *ptr,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+// Sized delete, C++14 only.
+#if __cplusplus >= 201402L
+inline void operator delete(void *ptr, __SIZE_TYPE__ size)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline void operator delete[](void *ptr, __SIZE_TYPE__ size) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+#endif
+
+#pragma pop_macro("OPENMP_NOEXCEPT")
+#endif
+
+#endif // include guard
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -144,6 +144,7 @@
   openmp_wrappers/cmath
   openmp_wrappers/__clang_openmp_math.h
   openmp_wrappers/__clang_openmp_math_declares.h
+  openmp_wrappers/new
 )
 
 set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)


Index: clang/lib/Headers/openmp_wrappers/new
===
--- /dev/null
+++ clang/lib/Headers/openmp_wrappers/new
@@ -0,0 +1,70 @@
+//===- new - OPENMP wrapper for  --===
+//
+// 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
+//
+//===---===
+
+#ifndef __CLANG_OPENMP_WRAPPERS_NEW
+#define __CLANG_OPENMP_WRAPPERS_NEW
+
+#include_next 
+
+#if defined(__NVPTX__) && defined(_OPENMP)
+
+#include 
+
+#pragma push_macro("OPENMP_NOEXCEPT")
+#if __cplusplus >= 201103L
+#define OPENMP_NOEXCEPT noexcept
+#else
+#define OPENMP_NOEXCEPT
+#endif
+
+// Device overrides for non-placement new and delete.
+inline void *operator new(__SIZE_TYPE__ size) {
+  if (size == 0)
+size = 1;
+  return ::malloc(size);
+}
+inline void *operator new(__SIZE_TYPE__ size,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  return ::operator new(size);
+}
+
+inline void *operator new[](__SIZE_TYPE__ size) { return ::operator new(size); }
+inline void *operator new[](__SIZE_TYPE__ size, const std::nothrow_t &) {
+  return ::operator new(size);
+}
+
+inline void operator delete(void *ptr)OPENMP_NOEXCEPT {
+  if (ptr)
+::free(ptr);
+}
+inline void operator delete(void *ptr, const std::nothrow_t &)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+inline void operator delete[](void *ptr) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline void operator delete[](void *ptr,
+  const std::nothrow_t &) OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+
+// Sized delete, C++14 only.
+#if __cplusplus >= 201402L
+inline void operator delete(void *ptr, __SIZE_TYPE__ size)OPENMP_NOEXCEPT {
+  ::operator delete(ptr);
+}
+inline vo

[clang] 569dc65 - [OPNEMP50][DOCS]Mark array shaping expression as claimed, NFC.

2020-02-05 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-02-05T10:02:39-05:00
New Revision: 569dc65c63802e2313aebe702d0233ae101c09cf

URL: 
https://github.com/llvm/llvm-project/commit/569dc65c63802e2313aebe702d0233ae101c09cf
DIFF: 
https://github.com/llvm/llvm-project/commit/569dc65c63802e2313aebe702d0233ae101c09cf.diff

LOG: [OPNEMP50][DOCS]Mark array shaping expression as claimed, NFC.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 39988a33715c..945f017183ee 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -225,7 +225,7 @@ implementation.
 
+--+--+--+---+
 | base language| lambda support
   | :good:`done` | 
  |
 
+--+--+--+---+
-| misc extension   | array shaping 
   | :none:`unclaimed`| 
  |
+| misc extension   | array shaping 
   | :part:`worked on`| 
  |
 
+--+--+--+---+
 | misc extension   | library shutdown (omp_pause_resource[_all])   
   | :none:`unclaimed parts`  | D55078  
  |
 
+--+--+--+---+



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


[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

2020-02-05 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 242607.
baloghadamsoftware added a comment.

Updated: checker moved to alpha.cplusplus (until at least one of the 
iterator-related checkers gets out of alpha state or a new non-alpha container 
checker is created). Error-branch is protected by an option. New constraint 
added for the non-error branch: the found value is not behind the parameter 
denoting the beginning of the lookup range.


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

https://reviews.llvm.org/D70818

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.h
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/stl-algorithm-modeling.cpp

Index: clang/test/Analysis/stl-algorithm-modeling.cpp
===
--- /dev/null
+++ clang/test/Analysis/stl-algorithm-modeling.cpp
@@ -0,0 +1,620 @@
+// RUN: %clang_analyze_cc1 -std=c++17 %s\
+// RUN:  -analyzer-checker=core,cplusplus,alpha.cplusplus.STLAlgorithmModeling,debug.DebugIteratorModeling,debug.ExprInspection\
+// RUN:  -analyzer-config aggressive-binary-operation-simplification=true\
+// RUN:  -analyzer-config alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling=true\
+// RUN:  -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void clang_analyzer_eval(bool);
+
+template 
+long clang_analyzer_iterator_position(const Iterator&);
+
+template  Iter return_any_iterator(const Iter &It);
+
+void test_find1(std::vector V, int n) {
+  const auto i1 = return_any_iterator(V.begin());
+  const auto i2 = return_any_iterator(V.begin());
+
+  const auto i3 = std::find(i1, i2, n);
+
+  clang_analyzer_eval(i3 == i2); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  if (i3 != i2) {
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{FALSE}}
+
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{FALSE}}
+  }
+}
+
+void test_find2(std::vector V, int n) {
+  const auto i1 = return_any_iterator(V.begin());
+  const auto i2 = return_any_iterator(V.begin());
+
+  const auto i3 = std::find(std::execution::sequenced_policy(), i1, i2, n);
+
+  clang_analyzer_eval(i3 == i2); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  if (i3 != i2) {
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{FALSE}}
+
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{FALSE}}
+  }
+}
+
+bool odd(int i) { return i % 2; }
+
+void test_find_if1(std::vector V) {
+  const auto i1 = return_any_iterator(V.begin());
+  const auto i2 = return_any_iterator(V.begin());
+
+  const auto i3 = std::find_if(i1, i2, odd);
+
+  clang_analyzer_eval(i3 == i2); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  if (i3 != i2) {
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i1)); // expected-warning@-1{{FALSE}}
+
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) <
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{TRUE}}
+clang_analyzer_eval(clang_analyzer_iterator_position(i3) >=
+clang_analyzer_iterator_position(i2)); // expected-warning@-1{{FALSE}}
+  }
+}
+
+void test_find_if2(std::vector V) {
+  const auto i1 = return_any_iterator(V.begin());
+  const auto i2 = return_any_iterator(V.begin());
+
+  const auto i3 = std::find_if(std::execution::sequenced_policy(), i1, i2, odd);
+
+  clang_analyzer_eval(i3 == i2); // expected-warn

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

2020-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I strongly agree with this comment: D70878#1780513 
, maybe the placement of functions 
like `getArg` and `getNumArgs` would be most appropriate in `CallDescription`. 
How about we try to cut down on duplicating functionalities?

I realize that many, if not all of my comments should've been placed at D70878 
, I was unfortunately not available then, but 
I think it would be a lot better if we settled on this before making the 
eventual changes too hard to switch. Checkers that were written with 
`CallDescriptionMap` (D70818 , D63915 
) or have recently been converted to it 
(D67706 , D62557 
, D68165 ) 
are a joy to look at, and reduce the overall complexity of the codebase 
greatly. Are there strong arguments against it?

The overall direction is great, as demonstrated by the test files. Its very 
exciting to see taintness analysis improving this much lately!




Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:103-132
   struct FunctionData {
 FunctionData() = delete;
 FunctionData(const FunctionData &) = default;
 FunctionData(FunctionData &&) = default;
 FunctionData &operator=(const FunctionData &) = delete;
 FunctionData &operator=(FunctionData &&) = delete;
 

I know this isn't really relevant, but isn't this redundant with 
`CallDescription`?



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:139
 
+  /// Add taint sources for extraction operator on pre-visit.
+  bool addOverloadedOpPre(const CallExpr *CE, CheckerContext &C) const;

Extraction operator? Is that a thing?



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:202
   using ConfigDataMap =
   std::unordered_multimap>;
   using NameRuleMap = ConfigDataMap;

http://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options

> We never use hash_set and unordered_set because they are generally very 
> expensive (each insertion requires a malloc) and very non-portable.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:373
+/// Treat implicit this parameter as the 0. argument.
+const Expr *getArg(const CallExpr *CE, unsigned ArgNum) {
+  if (const auto *MCE = dyn_cast(CE)) {

I would be shocked if this is the first time I've come across very similar 
function -- in any case, could you rename it to something like 
`getArgIgnoringImplicitThis`?



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:816
+bool GenericTaintChecker::isStdstream(const Expr *E, CheckerContext &C) {
+  llvm::Regex R{".*std::basic_istream.*"};
+  std::string Error;

In this case, maybe `llvm::Regex` is overkill?  
[[https://llvm.org/doxygen/classllvm_1_1StringRef.html#a82369bea2700347f68e1f43e30d2d47b
 | `llvm::StringRef::find()`]] may be sufficient.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71524



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


[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

2020-02-05 Thread Eugene Leviant via Phabricator via cfe-commits
evgeny777 accepted this revision.
evgeny777 added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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


[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

2020-02-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731
+  }
+  return C.getNoteTag([Text, Name](BugReport &BR) -> std::string {
+  SmallString<256> Msg;

Szelethus wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > You'll need to check whether the container is actually of interest to 
> > > > the bug report. We don't want notes to be added about changes to 
> > > > irrelevant containers.
> > > > 
> > > > You can use a combination of "Report `BR` was emitted by one of the 
> > > > iterator checkers" and "The memory region of the container is marked as 
> > > > interesting" (while also actually marking it as interesting in the 
> > > > checker).
> > > > 
> > > > Ideally we should instead make a new generic storage inside the 
> > > > `BugReport` object, in order to pass down the interesting information 
> > > > from the call site of `emitReport` ("Hi, i'm an iterator checker who 
> > > > emitted this report and i'm interested in changes made to the size of 
> > > > this container").
> > > Are you sure in this? I already wondered how it works so I added a test 
> > > that checks one container and changes another one and there were no note 
> > > tags displayed for the one we did not check but change. See the last test.
> > That's because you didn't do
> > ```lang=c++
> >   V2.cbegin();
> >   V2.cend();
> > ```
> > in the beginning.
> A similar conversation sparked up recently in between @boga95, @steakhal and 
> me regarding reporting taintedness. Bug reports are fine up to the point 
> where (in reverse) the first propagation happens, but finding out which value 
> tainted the one that caused the report isn't handled at the moment. One idea 
> was to mark the initial (again, in reverse) value as interesting, create a 
> `NoteTag` at the point of propagation, where we should know which value was 
> the cause of the spread, mark that interesting as well, etc.
> 
> If `NoteTag`s only emit a message when the concerning value is interesting, 
> this should theoretically solve that problem. I guess you could say that 
> we're propagating interestingness in reverse.
> 
> I'm not immediately sure if this idea was ever mentioned or implemented here.
Yes, that's the intended solution to such problems. `trackExpressionValue` 
works similarly, just with assignments instead of taint propagations. And in 
both cases note tags are a much more straightforward solution to the problem.


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

https://reviews.llvm.org/D73720



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


[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

2020-02-05 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked 3 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp:138-139
+  SVB.getConditionType());
+assert(Less.getAs() &&
+   "Symbol comparison must be a `DefinedSVal`");
+StateFound = StateFound->assume(Less.castAs(), true);

NoQ wrote:
> Is this because you only have atomic conjured symbols in your map? Because 
> otherwise the assertion will fail every time we reach a maximum symbol 
> complexity during `evalBinOp`.
> 
> I'd rather make the code defensive and handle the `UnknownVal` case. That 
> said, you can be sure it's not `UndefinedVal`.
In the map we either have atomic conjured symbols or an atomic conjured symbol 
plus/minus a concrete integer. It should not reach a maximum symbol complexity. 
The assertion is a copy from `IteratorModeling`.


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

https://reviews.llvm.org/D70818



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


[PATCH] D69582: Let clang driver support parallel jobs

2020-02-05 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/Compilation.cpp:332
+if (!Next) {
+  std::this_thread::yield();
   continue;

aganea wrote:
> yaxunl wrote:
> > aganea wrote:
> > > In addition to what @thakis said above, yielding here is maybe not a good 
> > > idea. This causes the process to spin, and remain in the OS' active 
> > > process list, which uselessly eats cpu cycles. This can become 
> > > significant over the course of several minutes of compilation.
> > > 
> > > Here's a //tiny// example of what happens when threads are waiting for 
> > > something to happen:
> > > (the top parts yields frequently; the bottom part does not yield - see 
> > > D68820)
> > > {F10592208}
> > > 
> > > You would need here to go through a OS primitive that suspends the 
> > > process until at least one job in the pool completes. On Windows this can 
> > > be achieved through `WaitForMultipleObjects()` or I/O completion ports 
> > > like provided by @thakis. You can take a look at 
> > > `Compilation::executeJobs()` in D52193 and further down the line, 
> > > `WaitMany()` which waits for at least one job/process to complete.
> > Sorry for the delay.
> > 
> > If D52193 is commited, I will probably only need some minor change to 
> > support parallel compilation for HIP. Therefore I hope D52193 could get 
> > committed soon.
> > 
> > I am wondering what is the current status of D52193 and what is blocking 
> > it. Is there any chance to get it commited soon?
> > 
> > Thanks.
> Hi @yaxunl! Nothing prevents from finishing D52193 :-) It was meant as a 
> prototype, but I could transform it into a more desirable state.
> I left it aside because we made another (unpublished) prototype, where the 
> process invocations were in fact collapsed into the calling process, ie. ran 
> in a thread pool in the manner of the recent `-fintegrated-cc1` flag. But 
> that requires for `cl::opt` to support different contexts, as opposed to just 
> one global state ([[ 
> http://lists.llvm.org/pipermail/llvm-dev/2018-October/127039.html | an RFC 
> was discussed ]] about a year ago, but there was no consensus).
> Having a thread pool instead of the process pool is faster when compiling 
> .C/.CPP files with `clang-cl /MP`, but perhaps in your case that won't work, 
> you need to call external binaries, do you? Binaries that are not part of 
> LLVM? If so, then landing D52193 first would makes sense.
HIP toolchain needs to launch executables other than clang for a compilation, 
therefore D52193 is more relevant to us. I believe this is also the case for 
CUDA, OpenMP and probably more general situations involving linker. I think 
both parallel by threads and parallel by processes are useful. However parallel 
by processes is probably more generic. Therefore landing D52193 first would 
benefit a lot.


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

https://reviews.llvm.org/D69582



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


[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

2020-02-05 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.

Thanks! Tests for the other value of the option are welcome (:


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

https://reviews.llvm.org/D70818



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


[clang] e63abde - [clang][driver] Fix null pointer dereference warning inside PrintActions1 (PR43462)

2020-02-05 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2020-02-05T15:32:18Z
New Revision: e63abde39f530028b0089ea477446c1b671a28f1

URL: 
https://github.com/llvm/llvm-project/commit/e63abde39f530028b0089ea477446c1b671a28f1
DIFF: 
https://github.com/llvm/llvm-project/commit/e63abde39f530028b0089ea477446c1b671a28f1.diff

LOG: [clang][driver] Fix null pointer dereference warning inside PrintActions1 
(PR43462)

As detailed on PR43462, clang static analyzer is complaining about a null 
pointer dereference as we provide a 'host' toolchain fallback if the ToolChain 
pointer is null, but then use that pointer anyhow to report the triple.

Tests indicate the ToolChain pointer is always valid and the 'host' code path 
is redundant.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 6db791ab8333..f35aab19e83d 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1848,6 +1848,7 @@ static unsigned PrintActions1(const Compilation &C, 
Action *A,
 bool IsFirst = true;
 OA->doOnEachDependence(
 [&](Action *A, const ToolChain *TC, const char *BoundArch) {
+  assert(TC && "Unknown host toolchain");
   // E.g. for two CUDA device dependences whose bound arch is sm_20 and
   // sm_35 this will generate:
   // "cuda-device" (nvptx64-nvidia-cuda:sm_20) {#ID}, "cuda-device"
@@ -1855,13 +1856,9 @@ static unsigned PrintActions1(const Compilation &C, 
Action *A,
   if (!IsFirst)
 os << ", ";
   os << '"';
-  if (TC)
-os << A->getOffloadingKindPrefix();
-  else
-os << "host";
+  os << A->getOffloadingKindPrefix();
   os << " (";
   os << TC->getTriple().normalize();
-
   if (BoundArch)
 os << ":" << BoundArch;
   os << ")";



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


[PATCH] D74046: [clang][driver] Fix null pointer dereference warning inside PrintActions1 (PR43462)

2020-02-05 Thread Simon Pilgrim via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe63abde39f53: [clang][driver] Fix null pointer dereference 
warning inside PrintActions1… (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74046

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1848,6 +1848,7 @@
 bool IsFirst = true;
 OA->doOnEachDependence(
 [&](Action *A, const ToolChain *TC, const char *BoundArch) {
+  assert(TC && "Unknown host toolchain");
   // E.g. for two CUDA device dependences whose bound arch is sm_20 and
   // sm_35 this will generate:
   // "cuda-device" (nvptx64-nvidia-cuda:sm_20) {#ID}, "cuda-device"
@@ -1855,13 +1856,9 @@
   if (!IsFirst)
 os << ", ";
   os << '"';
-  if (TC)
-os << A->getOffloadingKindPrefix();
-  else
-os << "host";
+  os << A->getOffloadingKindPrefix();
   os << " (";
   os << TC->getTriple().normalize();
-
   if (BoundArch)
 os << ":" << BoundArch;
   os << ")";


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1848,6 +1848,7 @@
 bool IsFirst = true;
 OA->doOnEachDependence(
 [&](Action *A, const ToolChain *TC, const char *BoundArch) {
+  assert(TC && "Unknown host toolchain");
   // E.g. for two CUDA device dependences whose bound arch is sm_20 and
   // sm_35 this will generate:
   // "cuda-device" (nvptx64-nvidia-cuda:sm_20) {#ID}, "cuda-device"
@@ -1855,13 +1856,9 @@
   if (!IsFirst)
 os << ", ";
   os << '"';
-  if (TC)
-os << A->getOffloadingKindPrefix();
-  else
-os << "host";
+  os << A->getOffloadingKindPrefix();
   os << " (";
   os << TC->getTriple().normalize();
-
   if (BoundArch)
 os << ":" << BoundArch;
   os << ")";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72746: [clangd] Add a flag for implicit references in the Index

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

thanks, looks good, a few nits.

before landing the patch, could you please run the clangd-indexer on the llvm 
project and measure the before/after indexing time? to make sure we don't 
introduce a big latency.
you can run the command like  `time ./bin/clangd-indexer -format=binary 
-executor=all-TUs . > static-index.idx`




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:588
 for (const auto &LocAndRole : IDAndRefs.second)
   CollectRef(IDAndRefs.first, LocAndRole);
   // Populate Refs slab from DeclRefs.

macro is tricky, macro references are not marked `spelled`, it is OK for now as 
we are not interested in them, but I think most of time they are spelled in the 
source code. Maybe add a comment?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:603
+// corresponding NamedDecl is. If it isn't, mark this reference as
+// implicit. An example of implicit reference (implicit = !spelled)
+// would be a macro expansion.

nit: the comment seems stale now.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:723
+  AllRanges.insert(end(AllRanges), begin(ImplicitRanges), end(ImplicitRanges));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(AllRanges;

nit: instead of checking all refs, I'd check implicit references explicitly, 
similar to the SpelledRefs below (just add a UnspelledSlab).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72746



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


[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2020-02-05 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

In D69825#1837243 , @hctim wrote:

> For tracking purposes.
>
> The ASan leaks have been worked around in two separate patches in order to 
> make the bots green again:
>
> - c38e42527b21acee8d01a016d5bfa2fb83202e29 
> 
> - e174da447c180b586719cb28f7bd556e30625762 
> 
>
>   @plotfi is graciously debugging :)


How is the debugging going? If it takes a while to fix, maybe it's better to 
make  -emit-interface-stub imply -fno-integrated-cc1 until it's fixed for real, 
instead of adding that in the tests? As-is, the tests are happy but the feature 
is likely broken (?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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


[clang] 91b3083 - [OpenCL] Fix tblgen support for cl_khr_mipmap_image_writes

2020-02-05 Thread Sven van Haastregt via cfe-commits

Author: Sven van Haastregt
Date: 2020-02-05T16:05:20Z
New Revision: 91b3083aecdcb7beb33d497a94f4467f110b4f6d

URL: 
https://github.com/llvm/llvm-project/commit/91b3083aecdcb7beb33d497a94f4467f110b4f6d
DIFF: 
https://github.com/llvm/llvm-project/commit/91b3083aecdcb7beb33d497a94f4467f110b4f6d.diff

LOG: [OpenCL] Fix tblgen support for cl_khr_mipmap_image_writes

Apply the fix of f780e15caf1 ("[OpenCL] Fix support for
cl_khr_mipmap_image_writes", 2020-01-27) also to the TableGen OpenCL
builtin function definitions.

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 0cbdc05700e9..888978dfdbd3 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -60,10 +60,11 @@ def FuncExtKhrLocalInt32ExtendedAtomics  : 
FunctionExtension<"cl_khr_local_int32
 def FuncExtKhrInt64BaseAtomics   : 
FunctionExtension<"cl_khr_int64_base_atomics">;
 def FuncExtKhrInt64ExtendedAtomics   : 
FunctionExtension<"cl_khr_int64_extended_atomics">;
 def FuncExtKhrMipmapImage: 
FunctionExtension<"cl_khr_mipmap_image">;
+def FuncExtKhrMipmapImageWrites  : 
FunctionExtension<"cl_khr_mipmap_image_writes">;
 def FuncExtKhrGlMsaaSharing  : 
FunctionExtension<"cl_khr_gl_msaa_sharing">;
 
 // Multiple extensions
-def FuncExtKhrMipmapAndWrite3d   : 
FunctionExtension<"cl_khr_mipmap_image cl_khr_3d_image_writes">;
+def FuncExtKhrMipmapWritesAndWrite3d : 
FunctionExtension<"cl_khr_mipmap_image_writes cl_khr_3d_image_writes">;
 
 // Qualified Type.  These map to ASTContext::QualType.
 class QualType {
@@ -1271,6 +1272,16 @@ let Extension = FuncExtKhrMipmapImage in {
   }
 }
   }
+  // Added to section 6.13.14.5
+  foreach aQual = ["RO", "WO", "RW"] in {
+foreach imgTy = [Image1d, Image2d, Image3d, Image1dArray, Image2dArray, 
Image2dDepth, Image2dArrayDepth] in {
+  def : Builtin<"get_image_num_mip_levels", [Int, ImageType]>;
+}
+  }
+}
+
+// Write functions are enabled using a separate extension.
+let Extension = FuncExtKhrMipmapImageWrites in {
   // Added to section 6.13.14.4.
   foreach aQual = ["WO"] in {
 foreach imgTy = [Image2d] in {
@@ -1295,7 +1306,7 @@ let Extension = FuncExtKhrMipmapImage in {
   def : Builtin<"write_imageui", [Void, ImageType, 
VectorType, Int, VectorType]>;
 }
 def : Builtin<"write_imagef", [Void, ImageType, 
VectorType, Int, Float]>;
-let Extension = FuncExtKhrMipmapAndWrite3d in {
+let Extension = FuncExtKhrMipmapWritesAndWrite3d in {
   foreach imgTy = [Image3d] in {
 def : Builtin<"write_imagef", [Void, ImageType, 
VectorType, Int, VectorType]>;
 def : Builtin<"write_imagei", [Void, ImageType, 
VectorType, Int, VectorType]>;
@@ -1303,15 +1314,8 @@ let Extension = FuncExtKhrMipmapImage in {
   }
 }
   }
-  // Added to section 6.13.14.5
-  foreach aQual = ["RO", "WO", "RW"] in {
-foreach imgTy = [Image1d, Image2d, Image3d, Image1dArray, Image2dArray, 
Image2dDepth, Image2dArrayDepth] in {
-  def : Builtin<"get_image_num_mip_levels", [Int, ImageType]>;
-}
-  }
 }
 
-
 //
 // OpenCL Extension v2.0 s18.3 - Creating OpenCL Memory Objects from OpenGL 
MSAA Textures
 let Extension = FuncExtKhrGlMsaaSharing in {



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


[PATCH] D73996: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

2020-02-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 242622.
lebedev.ri retitled this revision from "[Sema] Demote 'alignment is not a power 
of two' error into a warning" to "[Sema] Demote call-site-based 'alignment is a 
power of two' check for AllocAlignAttr into a warning".
lebedev.ri added a comment.

Scale ambitions down - only demote the call-site-based `AllocAlignAttr` error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73996

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
  clang/test/Sema/alloc-align-attr.c
  clang/test/SemaCXX/alloc-align-attr.cpp
  clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp

Index: clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
===
--- clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
+++ clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
@@ -32,7 +32,7 @@
 
 void *ptr_variable(int align) { return new (std::align_val_t(align)) A; }
 void *ptr_align16() { return new (std::align_val_t(16)) A; }
-void *ptr_align15() { return new (std::align_val_t(15)) A; } // expected-error {{requested alignment is not a power of 2}}
+void *ptr_align15() { return new (std::align_val_t(15)) A; } // expected-warning {{requested alignment is not a power of 2}}
 
 struct alignas(128) S {
   S() {}
@@ -49,7 +49,7 @@
   return new (std::align_val_t(256)) S;
 }
 void *alloc_overaligned_struct_with_extra_255_alignment(int align) {
-  return new (std::align_val_t(255)) S; // expected-error {{requested alignment is not a power of 2}}
+  return new (std::align_val_t(255)) S; // expected-warning {{requested alignment is not a power of 2}}
 }
 
 std::align_val_t align_variable(int align) { return std::align_val_t(align); }
Index: clang/test/SemaCXX/alloc-align-attr.cpp
===
--- clang/test/SemaCXX/alloc-align-attr.cpp
+++ clang/test/SemaCXX/alloc-align-attr.cpp
@@ -30,14 +30,14 @@
   dependent_ret b;
   b.Foo(1);
   b.Foo2(1);
-  b.Foo(3);   // expected-error {{requested alignment is not a power of 2}}
-  b.Foo2(3);  // expected-error {{requested alignment is not a power of 2}}
+  b.Foo(3);   // expected-warning {{requested alignment is not a power of 2}}
+  b.Foo2(3);  // expected-warning {{requested alignment is not a power of 2}}
   b.Foo(1073741824);  // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
   b.Foo2(1073741824); // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
   b.Foo(align);
   b.Foo2(align);
 
-  dependent_param_struct c; 
+  dependent_param_struct c;
   c.Foo(1);
   dependent_param_struct d; // expected-note {{in instantiation of template class 'dependent_param_struct' requested here}}
   d.Foo(1.0);
Index: clang/test/Sema/alloc-align-attr.c
===
--- clang/test/Sema/alloc-align-attr.c
+++ clang/test/Sema/alloc-align-attr.c
@@ -24,7 +24,7 @@
   return test_ptr_alloc_align(16);
 }
 void *align15() {
-  return test_ptr_alloc_align(15); // expected-error {{requested alignment is not a power of 2}}
+  return test_ptr_alloc_align(15); // expected-warning {{requested alignment is not a power of 2}}
 }
 void *align536870912() {
   return test_ptr_alloc_align(1073741824); // expected-warning {{requested alignment must be 536870912 bytes or smaller; maximum alignment assumed}}
Index: clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
===
--- /dev/null
+++ clang/test/CodeGen/non-power-of-2-alignment-assumptions.c
@@ -0,0 +1,46 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+void *__attribute__((alloc_align(1))) alloc(int align);
+
+// CHECK-LABEL: @t0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[ALIGN_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[ALIGN:%.*]], i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* [[ALIGN_ADDR]], align 4
+// CHECK-NEXT:[[CALL:%.*]] = call i8* @alloc(i32 [[TMP0]])
+// CHECK-NEXT:[[ALIGNMENTCAST:%.*]] = zext i32 [[TMP0]] to i64
+// CHECK-NEXT:[[MASK:%.*]] = sub i64 [[ALIGNMENTCAST]], 1
+// CHECK-NEXT:[[PTRINT:%.*]] = ptrtoint i8* [[CALL]] to i64
+// CHECK-NEXT:[[MASKEDPTR:%.*]] = and i64 [[PTRINT]], [[MASK]]
+// CHECK-NEXT:[[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0
+// CHECK-NEXT:call void @llvm.assume(i1 [[MASKCOND]])
+// CHECK-NEXT:ret void
+//
+void t0(int align) {
+  alloc(align);
+}
+// CHECK-LABEL: @

[clang] 3627c91 - [ARM][TargetParser] Improve handling of dependencies between target features

2020-02-05 Thread Momchil Velikov via cfe-commits

Author: Momchil Velikov
Date: 2020-02-05T16:07:51Z
New Revision: 3627c91ead934486fdb3986b911482a78f101309

URL: 
https://github.com/llvm/llvm-project/commit/3627c91ead934486fdb3986b911482a78f101309
DIFF: 
https://github.com/llvm/llvm-project/commit/3627c91ead934486fdb3986b911482a78f101309.diff

LOG: [ARM][TargetParser] Improve handling of dependencies between target 
features

The patch at https://reviews.llvm.org/D64048 added "negative"
dependency handling in `ARM::appendArchExtFeatures`: feature "noX"
removes all features, which imply "X".

This patch adds the "positive" handling: feature "X" adds all the
feature strings implied by "X".

(This patch also comes from the suggestion here
https://reviews.llvm.org/D72633#inline-658582)

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

Added: 


Modified: 
clang/lib/Basic/Targets/ARM.cpp
clang/test/Driver/arm-mfpu.c
clang/test/Preprocessor/arm-target-features.c
llvm/lib/Support/ARMTargetParser.cpp
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index c6f661fdec99..02144c6ebe85 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -480,10 +480,8 @@ bool 
ARMTargetInfo::handleTargetFeatures(std::vector &Features,
 } else if (Feature == "+dotprod") {
   DotProd = true;
 } else if (Feature == "+mve") {
-  DSP = 1;
   MVE |= MVE_INT;
 } else if (Feature == "+mve.fp") {
-  DSP = 1;
   HasLegalHalfType = true;
   FPU |= FPARMV8;
   MVE |= MVE_INT | MVE_FP;

diff  --git a/clang/test/Driver/arm-mfpu.c b/clang/test/Driver/arm-mfpu.c
index c3731fa5bd63..5309977d0f90 100644
--- a/clang/test/Driver/arm-mfpu.c
+++ b/clang/test/Driver/arm-mfpu.c
@@ -414,12 +414,14 @@
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-d32"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-neon"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-crypto"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+mve"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+dsp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-mve.fp"
 // CHECK-MVEFP-FPUNONE-NOT: "-target-feature" "-fpregs"
 
-
 // RUN: %clang -target arm-none-none-eabi %s -march=armv8.1-m.main+mve 
-mfpu=none -### -c 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MVEI-FPUNONE %s
 // CHECK-MVEI-FPUNONE-DAG: "-target-feature" "-mve.fp"
 // CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+mve"
+// CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+dsp"
 // CHECK-MVEI-FPUNONE-NOT: "-target-feature" "-fpregs"

diff  --git a/clang/test/Preprocessor/arm-target-features.c 
b/clang/test/Preprocessor/arm-target-features.c
index 3d80a1bd88cf..72b77b6124fe 100644
--- a/clang/test/Preprocessor/arm-target-features.c
+++ b/clang/test/Preprocessor/arm-target-features.c
@@ -761,9 +761,10 @@
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
 // CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1
 
-// nofp discards mve.fp
+// nofp discards mve.fp, but not mve/dsp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x 
c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVEFP-NOFP %s
-// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_MVE 1
 
 // nomve discards mve.fp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x 
c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVEFP-NOMVE %s
@@ -773,11 +774,16 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E 
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
 // CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1
 
-// nodsp discards both dsp and mve
+// nodsp discards both dsp and mve ...
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+nodsp -x c 
-E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVE-NODSP %s
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
 
+// ... and also mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nodsp -x 
c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVE-NODSP %s
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1

diff  --git a/llvm/lib/Support/ARMTargetParser.cpp 
b/llvm/lib/Support/ARMTargetParser.cpp
index 360e64a5a5f4..3c3c2103a5c1 100644
--- a/llvm/lib/Support/ARMTargetParser.cpp
+++ b/llvm/lib/Support/ARMTargetParser.cpp
@@ -498,10 +498,13 @@ bool AR

[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

2020-02-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Fixes https://github.com/clangd/clangd/issues/277


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74054

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -45,6 +45,11 @@
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,15 +665,38 @@
Sym("baz", T.range("StaticOverload2";
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-template  struct function {};
-template  using callback = function;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+R"cpp(
+  template  struct [[function]] {};
+  template  using [[callback]] = function;
 
-c^allback foo;
-  )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+  c^allback foo;
+)cpp",
+
+R"cpp(
+  class [[Foo]] {};
+  typedef Foo [[Ba^r]];
+)cpp",
+
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::[[F^oo]];
+)cpp",
+
+R"cpp(
+  class [[Foo]] {};
+  using [[B^ar]] = Foo;
+)cpp",
+  };
+
+  for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -273,8 +273,8 @@
   };
 
   // Emit all symbol locations (declaration or definition) from AST.
-  DeclRelationSet Relations =
-  DeclRelation::TemplatePattern | DeclRelation::Alias;
+  DeclRelationSet Relations = DeclRelation::TemplatePattern |
+  DeclRelation::Alias | DeclRelation::Underlying;
   for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
 // Special case: void foo() ^override: jump to the overridden method.
 if (const auto *CMD = llvm::dyn_cast(D)) {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -45,6 +45,11 @@
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
+MATCHER(DeclRange, "") {
+  const LocatedSymbol &Sym = ::testing::get<0>(arg);
+  const Range &Range = ::testing::get<1>(arg);
+  return Sym.PreferredDeclaration.range == Range;
+}
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
@@ -660,15 +665,38 @@
Sym("baz", T.range("StaticOverload2";
 }
 
-TEST(LocateSymbol, TemplateTypedefs) {
-  auto T = Annotations(R"cpp(
-template  struct function {};
-template  using callback = function;
+TEST(LocateSymbol, Alias) {
+  const char *Tests[] = {
+R"cpp(
+  template  struct [[function]] {};
+  template  using [[callback]] = function;
 
-c^allback foo;
-  )cpp");
-  auto AST = TestTU::withCode(T.code()).build();
-  EXPECT_THAT(locateSymbolAt(AST, T.point()), ElementsAre(Sym("callback")));
+  c^allback foo;
+)cpp",
+
+R"cpp(
+  class [[Foo]] {};
+  typedef Foo [[Ba^r]];
+)cpp",
+
+R"cpp(
+  namespace ns { class [[Foo]] {}; }
+  using ns::[[F^oo]];
+)cpp",
+
+R"cpp(
+  class [[Foo]] {};
+  using [[B^ar]] = Foo;
+)cpp",
+  };
+
+  for (const auto* Case : Tests) {
+SCOPED_TRACE(Case);
+auto T = Annotations(Case);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(locateSymbolAt(AST, T.point()),
+::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+  }
 }
 
 TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: clang-tools-extra/clangd/XRefs.cpp
===

[PATCH] D74048: [SYCL][Driver] Add clang driver option to enable SYCL compilation mode

2020-02-05 Thread Alexey Bader via Phabricator via cfe-commits
bader updated this revision to Diff 242627.
bader added a comment.

Applied suggestions from Alexey


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74048

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/sycl.c


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s 
--check-prefix=ENABLED
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s 
--check-prefix=DISABLED
+// RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+// ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// DISABLED-NOT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,6 +4023,15 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = false;
+
+  Arg *SYCLArg = Args.getLastArg(options::OPT_fsycl, options::OPT_fno_sycl);
+  if (SYCLArg)
+IsSYCL = SYCLArg->getOption().matches(options::OPT_fsycl);
+
+  if (IsSYCL)
+CmdArgs.push_back("-fsycl-is-device");
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP 
device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3407,6 +3410,11 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group,
+  HelpText<"Disable SYCL kernels compilation for device">;
 
 include "CC1Options.td"
 


Index: clang/test/Driver/sycl.c
===
--- /dev/null
+++ clang/test/Driver/sycl.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+// ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// DISABLED-NOT: "-fsycl-is-device"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4023,6 +4023,15 @@
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
   }
 
+  bool IsSYCL = false;
+
+  Arg *SYCLArg = Args.getLastArg(options::OPT_fsycl, options::OPT_fno_sycl);
+  if (SYCLArg)
+IsSYCL = SYCLArg->getOption().matches(options::OPT_fsycl);
+
+  if (IsSYCL)
+CmdArgs.push_back("-fsycl-is-device");
+
   if (IsOpenMPDevice) {
 // We have to pass the triple of the host if compiling for an OpenMP device.
 std::string NormalizedTriple =
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -124,6 +124,9 @@
 def opencl_Group : OptionGroup<"">, Group,
DocName<"OpenCL flags">;
 
+def sycl_Group : OptionGroup<"">, Group,
+ DocName<"SYCL flags">;
+
 def m_Group : OptionGroup<"">, Group,
   DocName<"Target-dependent compilation options">;
 
@@ -3407,6 +3410,11 @@
 defm underscoring : BooleanFFlag<"underscoring">, Group;
 defm whole_file : BooleanFFlag<"whole-file">, Group;
 
+// C++ SYCL options
+def fsycl : Flag<["-"], "fsycl">, Group,
+  HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Grou

[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

2020-02-05 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added a comment.

I have updated the TableGen OpenCL builtin definitions accordingly in 
91b3083aecd 
 
("[OpenCL] Fix tblgen support for cl_khr_mipmap_image_writes", 2020-02-05).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71460



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


[PATCH] D72762: [ARM][TargetParser] Improve handling of dependencies between target features

2020-02-05 Thread Momchil Velikov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3627c91ead93: [ARM][TargetParser] Improve handling of 
dependencies between target features (authored by chill).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72762

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Driver/arm-mfpu.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -637,6 +637,27 @@
   }
 }
 
+static bool
+testArchExtDependency(const char *ArchExt,
+  const std::initializer_list &Expected) {
+  std::vector Features;
+
+  if (!ARM::appendArchExtFeatures("", ARM::ArchKind::ARMV8_1MMainline, ArchExt,
+  Features))
+return false;
+
+  return llvm::all_of(Expected, [&](StringRef Ext) {
+return llvm::is_contained(Features, Ext);
+  });
+}
+
+TEST(TargetParserTest, ARMArchExtDependencies) {
+  EXPECT_TRUE(testArchExtDependency("mve", {"+mve", "+dsp"}));
+  EXPECT_TRUE(testArchExtDependency("mve.fp", {"+mve.fp", "+mve", "+dsp"}));
+  EXPECT_TRUE(testArchExtDependency("nodsp", {"-dsp", "-mve", "-mve.fp"}));
+  EXPECT_TRUE(testArchExtDependency("nomve", {"-mve", "-mve.fp"}));
+}
+
 TEST(TargetParserTest, ARMparseHWDiv) {
   const char *hwdiv[] = {"thumb", "arm", "arm,thumb", "thumb,arm"};
 
Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -498,10 +498,13 @@
 return false;
 
   for (const auto AE : ARCHExtNames) {
-if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
-  Features.push_back(AE.NegFeature);
-else if (AE.ID == ID && AE.Feature)
-  Features.push_back(AE.Feature);
+if (Negated) {
+  if ((AE.ID & ID) == ID && AE.NegFeature)
+Features.push_back(AE.NegFeature);
+} else {
+  if ((AE.ID & ID) == AE.ID && AE.Feature)
+Features.push_back(AE.Feature);
+}
   }
 
   if (CPU == "")
Index: clang/test/Preprocessor/arm-target-features.c
===
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -761,9 +761,10 @@
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
 // CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1
 
-// nofp discards mve.fp
+// nofp discards mve.fp, but not mve/dsp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOFP %s
-// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_MVE 1
 
 // nomve discards mve.fp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOMVE %s
@@ -773,11 +774,16 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
 // CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1
 
-// nodsp discards both dsp and mve
+// nodsp discards both dsp and mve ...
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
 
+// ... and also mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: clang/test/Driver/arm-mfpu.c
===
--- clang/test/Driver/arm-mfpu.c
+++ clang/test/Driver/arm-mfpu.c
@@ -414,12 +414,14 @@
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-d32"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-neon"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-crypto"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+mve"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+dsp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-mve.fp"
 // CH

  1   2   >