date:20200509

[PATCH] D79653: [sanitizer] Enable whitelist/blacklist in new PM

2020-05-09 Thread Jinsong Ji via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa72b9dfd45cd: [sanitizer] Enable whitelist/blacklist in new 
PM (authored by jsji).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79653

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cpp
  llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp


Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -322,7 +322,8 @@
 
 PreservedAnalyses ModuleSanitizerCoveragePass::run(Module &M,
ModuleAnalysisManager &MAM) 
{
-  ModuleSanitizerCoverage ModuleSancov(Options);
+  ModuleSanitizerCoverage ModuleSancov(Options, Whitelist.get(),
+   Blacklist.get());
   auto &FAM = 
MAM.getResult(M).getManager();
   auto DTCallback = [&FAM](Function &F) -> const DominatorTree * {
 return &FAM.getResult(F);
Index: llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
===
--- llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
+++ llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
@@ -18,6 +18,7 @@
 
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Transforms/Instrumentation.h"
 
 namespace llvm {
@@ -30,12 +31,26 @@
 : public PassInfoMixin {
 public:
   explicit ModuleSanitizerCoveragePass(
-  SanitizerCoverageOptions Options = SanitizerCoverageOptions())
-  : Options(Options) {}
+  SanitizerCoverageOptions Options = SanitizerCoverageOptions(),
+  const std::vector &WhitelistFiles =
+  std::vector(),
+  const std::vector &BlacklistFiles =
+  std::vector())
+  : Options(Options) {
+if (WhitelistFiles.size() > 0)
+  Whitelist = SpecialCaseList::createOrDie(WhitelistFiles,
+   *vfs::getRealFileSystem());
+if (BlacklistFiles.size() > 0)
+  Blacklist = SpecialCaseList::createOrDie(BlacklistFiles,
+   *vfs::getRealFileSystem());
+  }
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
 private:
   SanitizerCoverageOptions Options;
+
+  std::unique_ptr Whitelist;
+  std::unique_ptr Blacklist;
 };
 
 // Insert SanitizerCoverage instrumentation.
Index: 
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cpp
===
--- 
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cpp
+++ 
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cpp
@@ -51,6 +51,13 @@
 // RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fsanitize-coverage-whitelist=wl_foo.txt  
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
 // RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fsanitize-coverage-whitelist=wl_bar.txt  
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
 
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fexperimental-new-pass-manager   
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fexperimental-new-pass-manager -fsanitize-coverage-whitelist=wl_all.txt  
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fexperimental-new-pass-manager -fsanitize-coverage-whitelist=wl_none.txt 
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fexperimental-new-pass-manager -fsanitize-coverage-whitelist=wl_file.txt 
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f patterns.txt
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm 
-fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table 
-fexperimental-new-pass-manager -fsanitize-coverage-whitelist=wl_foo.txt  
-fsanitize-coverage-blacklist=bl_all.txt   2>&1 | not grep -f

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

In D79636#2028479 , @efriedma wrote:

> > @efriedma I'm a bit confused. Could you propose some wording so I get a 
> > feeling where you want to go?
>
> Depending on which direction we go, either:
>
> - "Attributes on a function or callsite describe the behavior of the callee 
> excluding the implied copy.  For example, an optimization can infer readnone 
> on a a byval argument if the callee does not access the copied memory."
> - "Attributes on a function or callsite describe the behavior of a call 
> including the implied copy.  For example, an optimization can never infer 
> readnone on a call with a byval argument readnone because the implied copy 
> reads memory.  byval implies nocapture: there isn't any way to retrieve the 
> original address in the callee."
>
>   I don't really care which we choose, we can easily model it either way.  
> But I think we need to choose one or the other.  Making attributes describe 
> different things on each side of the call is much worse than either of those 
> alternatives; it confuses what it means for a function or callsite to have an 
> attribute.


I think this helped. Thanks. I also understand your concerns now.

Given the choice between the two, I vote for the second:
Pros:

- There is a place the read is attributed to, thus no need for D79454 
.
- The `byval` -> `nocapture` step at the call site can be useful (done already 
by the Attributor).
- It also is probably important to have `byval` -> `noalias` in the callee.

Cons:

- You don't get `byval` -> `readonly` at the call site anymore (done by the 
Attributor).
- Alignment at the call site might be higher than of the copy, breaking with 
the idea that the call site and callee "properties" match. Though, the 
attributes can probably be kept in sync if we teach the relevant parts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> @efriedma I'm a bit confused. Could you propose some wording so I get a 
> feeling where you want to go?

Depending on which direction we go, either:

- "Attributes on a function or callsite describe the behavior of the callee 
excluding the implied copy.  For example, an optimization can infer readnone on 
a a byval argument if the callee does not access the copied memory."
- "Attributes on a function or callsite describe the behavior of a call 
including the implied copy.  For example, an optimization can never infer 
readnone on a call with a byval argument readnone because the implied copy 
reads memory.  byval implies nocapture: there isn't any way to retrieve the 
original address in the callee."

I don't really care which we choose, we can easily model it either way.  But I 
think we need to choose one or the other.  Making attributes describe different 
things on each side of the call is much worse than either of those 
alternatives; it confuses what it means for a function or callsite to have an 
attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

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

Sorry i'm not very responsive these days >.<




Comment at: 
clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp:95
   ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+return;

xazax.hun wrote:
> I think this change might be a better fit for a separate commit. I think you 
> don't even need to have such a small change reviewed. You could just commit 
> it as is. Just do not forget to describe that these cases are really hard to 
> have a test for but this is the idiomatic way of doing this in checkers.
I'm also curious why did this suddenly become necessary. This is obviously the 
right thing to do but why the change? It might indicate that we accidentally 
started incorrectly agglutinating nodes that were previously considered 
different. Like, we might have messed up our program points somewhere in this 
patch.



Comment at: clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp:31-32
 
-  // FIXME: All calls to operator new should be CXXAllocatorCall.
-  // FIXME: PostStmt should be present.
+  // FIXME: All calls to operator new should be CXXAllocatorCall, and calls to
+  // operator delete should be CXXDeallocatorCall.
   {

That's fairly unobvious to me. Isn't the whole point of 
`CXXAllocatorCall`/`CXXDeallocatorCall` to give access to specific aspects of 
new/delete-expressions that aren't available in manual invocations with 
function syntax? On the other hand, i agree that it's nice to be able to cast 
the call event to `CXXAllocatorCall`/`CXXDeallocatorCall` and be done with 
allocators/deallocators.


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

https://reviews.llvm.org/D75430



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

[PATCH] D79232: [analyzer] Refactor range inference for symbolic expressions

2020-05-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Analysis/constant-folding.c:127-128
+  if (a > 10) {
+clang_analyzer_eval((a & 1) <= 1); // expected-warning{{FALSE}}
+clang_analyzer_eval((a & 1) > 1);  // expected-warning{{FALSE}}
+  }

vsavchenko wrote:
> NoQ wrote:
> > vsavchenko wrote:
> > > NoQ wrote:
> > > > vsavchenko wrote:
> > > > > NoQ wrote:
> > > > > > How can both of these be false? o.o
> > > > > Yeah :) I realized how weird it is.
> > > > > Anything is possible in the land of infeasible ranges.
> > > > > 
> > > > > I changed a comment there to address this
> > > > I mean, this pretty much never happened before. How are you not 
> > > > tripping on [[ 
> > > > https://github.com/llvm/llvm-project/blob/1a4421a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h#L100
> > > >  | this assert ]]? (probably it's simply been disabled in normal debug 
> > > > builds now that it's under "expensive checks")
> > > > 
> > > > The correct thing to do is to detect the paradox earlier and mark the 
> > > > path as infeasible. What prevents us from doing it right away here?
> > > Before we didn't really care about constraints on the operands and I 
> > > changed it :)
> > > So, now `Intersect` (which is logically not a correct way to do what is 
> > > meant) can cause this type of behaviour
> > [visible confusion]
> > 
> > Could you elaborate? I see that only constraint so far is `$a: [11; 
> > UINT_MAX]`. I don't see any infeasible ranges here. `(a & 1) <= 1` is 
> > clearly true. If we were previously thinking that it's unknown and now we 
> > think that it's false, then it's a regression.
> `a` is indeed `[11, UINT_MAX]`.
> Current implementation checks a constant (i.e. `1`) and intersects the range 
> for LHS `[11, UINT_MAX]` with `[UINT_MIN, 1]`, which produces empty range set 
> (aka infeasible).
> 
> This is why I'm saying that intersection is a bad choice, it's even plain 
> wrong.
> Before this patch we ignored constraints for `a` and considered it to be 
> `[UINT_MIN, UINT_MAX]`. In that setting, intersection does indeed work (which 
> doesn't make it correct).
> 
> Yes, it is a regression. I'm changing this implementation in the child 
> revisions.
> 
> 
> Yes, it is a regression. I'm changing this implementation in the child 
> revisions.

Oh, right, got it :D

Ok, let's land 'em together then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79232



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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

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



Comment at: llvm/docs/LangRef.rst:1052-1053
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, which is the call site argument, is always read. This

arsenm wrote:
> Should this also specify the meaning of readonly/readnone as a callee side 
> parameter attribute? is it disallowed to write to a readonly byval argument?
> Should this also specify the meaning of readonly/readnone as a callee side 
> parameter attribute?

I thought by specifying what memory the call site and argument pointer refer 
to, the readonly/readnone (and other attributes) fall in line. They apply to 
the respective memory.

> is it disallowed to write to a readonly byval argument?

Writing *any* `readonly` argument is UB, I mean the store instruction causes UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

In D79636#2028294 , @efriedma wrote:

> > Do you object to say that the call site argument and the argument point to 
> > distinct memory locations or something else?
>
> Like I said, my issue is with the "Attributes on the call site argument and 
> function argument are associated with the original and copied memory 
> respectively".  I assume this means "attributes other than byval".

Yes. `byval` applies to both.

> If I'm understanding this correctly, this means it isn't legal to copy 
> attributes from the caller to the callee.

Correct. It is not legal to do that now either, see below, but we just don't 
say so. TBH, I have not heard a proposal in which it would be legal but the 
copy would still happen implicitly (somewhere).

> If an argument is marked "readnone byval" on a function, it's illegal to copy 
> that "readnone" to the callsite, because the readnone would then be 
> associated with the original memory, not the copied memory.

Right. That is what I think needs to be the semantics.

> Or, a more silly example, say you had "byval returned" on the called 
> function, and that got copied to the callsite: that clearly can't mean the 
> original pointer is returned by the function.

I think `byval returned` example shows nicely that we cannot copy the 
attributes, right?

@efriedma I'm a bit confused. Could you propose some wording so I get a feeling 
where you want to go?

In D79636#2028363 , @aqjune wrote:

> I have a minor question:
>
> > a call of a readnone function with a byval argument is not classified as 
> > readnone (which it is today: https://godbolt.org/z/dDfQ5r)
>
> %0 at caller has readnone attribute - is it related with the propagation of 
> readnone attribute from %0 of empty function to the caller?
>  Some comments above seems to be related with this question, but I rather 
> wonder about the validity of the propagation of readnone in this example.

The propagation in this example is *not* valid. This patch makes this clear (I 
hope).

> Actually I wonder whether things will become clearer if an alloca-and-copy 
> (or something that is equivalent with this) is explicitly used to show the 
> behavior of pass-as-value rather than byval implicitly encoding the behavior; 
> my impression is that byval is different from other attributes like readnone 
> or nonnull, because it isn't the result of value analysis. This will be a lot 
> of work though...

You are not wrong. Making it explicit would actually help us. I am in favor. 
Nonetheless, we currently seem to have no clear semantics on what `byval` means 
and how it interacts with other attributes. Clang strips 
`__attribute__((pure))` if `byval` arguments are present but `functionattrs` 
will just add it again. More generally, this does currently not work:

  for (Instruction *I : instructions(Fn))
MayReadOrWrite |= I->mayReadOrWriteMemory();

if a call takes a `byval` argument.

Long story short, I would prefer this change to make the current behavior 
consistent and then a transition away from `byval` to some explicit copy model.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636

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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added inline comments.



Comment at: llvm/docs/LangRef.rst:1052-1053
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, which is the call site argument, is always read. This

Should this also specify the meaning of readonly/readnone as a callee side 
parameter attribute? is it disallowed to write to a readonly byval argument?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Juneyoung Lee via Phabricator via cfe-commits

aqjune added a comment.

I have a minor question:

> a call of a readnone function with a byval argument is not classified as 
> readnone (which it is today: https://godbolt.org/z/dDfQ5r)

%0 at caller has readnone attribute - is it related with the propagation of 
readnone attribute from %0 of empty function to the caller?
Some comments above seems to be related with this question, but I rather wonder 
about the validity of the propagation of readnone in this example.

Actually I wonder whether things will become clearer if an alloca-and-copy (or 
something that is equivalent with this) is explicitly used to show the behavior 
of pass-as-value rather than byval implicitly encoding the behavior; my 
impression is that byval is different from other attributes like readnone or 
nonnull, because it isn't the result of value analysis. This will be a lot of 
work though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

2020-05-09 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b02be0b973a: [Clang] Pass -z max-page-size to linker for 
Fuchsia (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D79667?vs=263004&id=263037#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79667

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -47,6 +47,9 @@
   Args.ClaimAllArgs(options::OPT_w);
 
   CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
+  CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -47,6 +47,9 @@
   Args.ClaimAllArgs(options::OPT_w);
 
   CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
+  CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

2020-05-09 Thread Petr Hosek via Phabricator via cfe-commits

phosek accepted this revision.
phosek 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/D79667/new/

https://reviews.llvm.org/D79667



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

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

2020-05-09 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8fbcb1e78ad: [Clang] Pass --pack-dyn-relocs=relr to lld for 
Fuchsia (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79665

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79678: [clangd] Add CSV export for trace metrics

2020-05-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Example: 
https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79678

Files:
  clang-tools-extra/clangd/support/Trace.cpp
  clang-tools-extra/clangd/support/Trace.h
  clang-tools-extra/clangd/test/metrics.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/support/TraceTests.cpp

Index: clang-tools-extra/clangd/unittests/support/TraceTests.cpp
===
--- clang-tools-extra/clangd/unittests/support/TraceTests.cpp
+++ clang-tools-extra/clangd/unittests/support/TraceTests.cpp
@@ -11,6 +11,7 @@
 #include "support/Trace.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/Threading.h"
@@ -22,7 +23,11 @@
 namespace clangd {
 namespace {
 
+using testing::_;
+using testing::ElementsAre;
+using testing::MatchesRegex;
 using testing::SizeIs;
+using testing::StartsWith;
 
 MATCHER_P(StringNode, Val, "") {
   if (arg->getType() != llvm::yaml::Node::NK_Scalar) {
@@ -138,6 +143,49 @@
   EXPECT_THAT(Tracer.takeMetric(MetricName, OpName), SizeIs(1));
 }
 
+class CSVMetricsTracerTest : public ::testing::Test {
+protected:
+  CSVMetricsTracerTest()
+  : OS(Output), Tracer(trace::createCSVMetricTracer(OS)), Session(*Tracer) {
+  }
+  trace::Metric Dist = {"dist", trace::Metric::Distribution, "lbl"};
+  trace::Metric Counter = {"cnt", trace::Metric::Counter};
+
+  std::string Output;
+  llvm::raw_string_ostream OS;
+  std::unique_ptr Tracer;
+  trace::Session Session;
+};
+
+TEST_F(CSVMetricsTracerTest, RecordsValues) {
+  Dist.record(1, "x");
+  Counter.record(1, "");
+  Dist.record(2, "y");
+
+  // Deliberately don't flush output stream, the tracer should do that.
+  // This is important when clangd crashes.
+  llvm::SmallVector Lines;
+  llvm::StringRef(Output).split(Lines, "\r\n");
+  EXPECT_THAT(
+  Lines,
+  ElementsAre("Kind,Metric,Label,Value,Timestamp",
+  MatchesRegex(R"(d,dist,x,1\.00e\+00,[0-9]+\.[0-9]{6})"),
+  StartsWith("c,cnt,,1.00e+00,"),
+  StartsWith("d,dist,y,2.00e+00,"), ""));
+}
+
+TEST_F(CSVMetricsTracerTest, Escaping) {
+  Dist.record(1, ",");
+  Dist.record(1, "a\"b");
+  Dist.record(1, "a\nb");
+
+  llvm::SmallVector Lines;
+  llvm::StringRef(Output).split(Lines, "\r\n");
+  EXPECT_THAT(Lines, ElementsAre(_, StartsWith(R"(d,dist,",",1)"),
+ StartsWith(R"(d,dist,"a""b",1)"),
+ StartsWith("d,dist,\"a\nb\",1"), ""));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -539,18 +539,23 @@
   // Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a
   // trace flag in your editor's config is annoying, launching with
   // `CLANGD_TRACE=trace.json vim` is easier.
-  llvm::Optional TraceStream;
+  llvm::Optional TracerStream;
   std::unique_ptr Tracer;
-  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
+  const char *JSONTraceFile = getenv("CLANGD_TRACE");
+  const char *MetricsCSVFile = getenv("CLANGD_METRICS");
+  const char *TracerFile = JSONTraceFile ? JSONTraceFile : MetricsCSVFile;
+  if (TracerFile) {
 std::error_code EC;
-TraceStream.emplace(TraceFile, /*ref*/ EC,
-llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
+TracerStream.emplace(TracerFile, /*ref*/ EC,
+ llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
 if (EC) {
-  TraceStream.reset();
-  llvm::errs() << "Error while opening trace file " << TraceFile << ": "
+  TracerStream.reset();
+  llvm::errs() << "Error while opening trace file " << TracerFile << ": "
<< EC.message();
 } else {
-  Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
+  Tracer = (TracerFile == JSONTraceFile)
+   ? trace::createJSONTracer(*TracerStream, PrettyPrint)
+   : trace::createCSVMetricTracer(*TracerStream);
 }
   }
 
Index: clang-tools-extra/clangd/test/metrics.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/metrics.test
@@ -0,0 +1,11 @@
+# RUN: env CLANGD_METRICS=%t clangd -lit-test < %s && FileCheck %s < %t
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

2020-05-09 Thread Petr Hosek via Phabricator via cfe-commits

phosek accepted this revision.
phosek 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/D79665/new/

https://reviews.llvm.org/D79665



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

[clang] 5b02be0 - [Clang] Pass -z max-page-size to linker for Fuchsia

2020-05-09 Thread Petr Hosek via cfe-commits


Author: Petr Hosek
Date: 2020-05-09T13:44:20-07:00
New Revision: 5b02be0b973a0d792bf8ce39170487f48b6cbd08

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

LOG: [Clang] Pass -z max-page-size to linker for Fuchsia

Currently all Fuchsia ABIs use a 4k page size, departing from
the recommended page sizes in the respective psABI documents.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 9a8f14de5c07..8f485802887f 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@ void fuchsia::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 3aceba70d7f8..359a2797da21 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"



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

[clang] c8fbcb1 - [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

2020-05-09 Thread Petr Hosek via cfe-commits


Author: Petr Hosek
Date: 2020-05-09T13:42:19-07:00
New Revision: c8fbcb1e78adcbaeadca9db9188771d81f49493a

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

LOG: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

The compact format is fully supported on Fuchsia and is the
preferred default.

Patch By: mcgrathr

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index fbda60ac7a45..9a8f14de5c07 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@ void fuchsia::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 5c1c00061d3a..3aceba70d7f8 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"



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

[PATCH] D79677: [clang][OpenMP][OMPIRBuilder] Adding some Privatization clauses to OpenMP `parallel` Directive

2020-05-09 Thread Fady Ghanim via Phabricator via cfe-commits

fghanim created this revision.
fghanim added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, guansong, yaxunl.
Herald added a project: clang.
fghanim added a parent revision: D79676: [Clang][OpenMP][OMPBuilder] Moving OMP 
allocation and cache creation code to OMPBuilderCBHelpers.

- Added support for Codegen `private` clause
- Added support for Codegen `firstprivate` Clause
- Added support for CodeGen of `copyin` Clause
- Added/moved code to support above tasks on the OMPIRBuilder


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79677

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_private_codegen.cpp

Index: clang/test/OpenMP/parallel_private_codegen.cpp
===
--- clang/test/OpenMP/parallel_private_codegen.cpp
+++ clang/test/OpenMP/parallel_private_codegen.cpp
@@ -1,8 +1,9 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefixes=ALL,CHECK
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s -check-prefixes=ALL,CHECK
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++11 -DLAMBDA -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck -check-prefix=LAMBDA %s
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -fblocks -DBLOCKS -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck -check-prefix=BLOCKS %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefixes=ALL,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
@@ -91,12 +92,12 @@
   }
 };
 
-// CHECK: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
+// ALL: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
 // LAMBDA: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
 // BLOCKS: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
-// CHECK: [[S_FLOAT_TY:%.+]] = type { float }
-// CHECK: [[S_INT_TY:%.+]] = type { i{{[0-9]+}} }
-// CHECK: [[SST_TY:%.+]] = type { i{{[0-9]+}} }
+// ALL: [[S_FLOAT_TY:%.+]] = type { float }
+// ALL: [[S_INT_TY:%.+]] = type { i{{[0-9]+}} }
+// ALL: [[SST_TY:%.+]] = type { i{{[0-9]+}} }
 template 
 T tmain() {
   S test;
@@ -273,63 +274,93 @@
 #endif
 }
 
-// CHECK: define i{{[0-9]+}} @main()
-// CHECK: [[TEST:%.+]] = alloca [[S_FLOAT_TY]],
-// CHECK: call {{.*}} [[S_FLOAT_TY_DEF_CONSTR:@.+]]([[S_FLOAT_TY]]* [[TEST]])
-// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 0, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*)* [[MAIN_MICROTASK:@.+]] to void
-// CHECK: = call i{{.+}} [[TMAIN_INT:@.+]]()
-// CHECK: call void [[S_FLOAT_TY_DESTR:@.+]]([[S_FLOAT_TY]]*
-// CHECK: ret
+// ALL: define i{{[0-9]+}} @main()
+// ALL: [[TEST:%.+]] = alloca [[S_FLOAT_TY]],
+// ALL: call {{.*}} [[S_FLOAT_TY_DEF_CONSTR:@.+]]([[S_FLOAT_TY]]* [[TEST]])
+// ALL: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 0, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*)* [[MAIN_MICROTASK:@.+]] to void
+// ALL: = call i{{.+}} [[TMAIN_INT:@.+]]()
+// ALL: call void [[S_FLOAT_TY_DESTR:@.+]]([[S_FLOAT_TY]]*
+// ALL: ret
 //
-// CHECK: define internal void [[MAIN_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}})
-// CHECK: [[T_VAR_PRIV:%.+]] = alloca i{{[0-9]+}},
-// CHECK: [[VEC_PRIV:%.+]] = alloca [2 x i{{[0-9]+}}],
-// CHECK: [[S_ARR_PRIV:%.+]] = alloca [2 x [[S_FLOAT_TY]]],
-// CHECK: [[VAR_PRIV:%.+]] = alloca [[S_FLOAT_TY]],
-// CHECK: [[SIVAR_PRIV:%.+]] = alloca i{{[0-9]+}},
+// ALL: define internal void [[MAIN_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}})
+// ALL: [[T_VAR_PRIV:%.+]] = alloca i{{[0-9]+}},
+// ALL: [[VEC_PRIV:%.+]] = alloca [2 x i{{[0-9]+}}],
+// ALL: [[S_ARR_PRIV:%.+]] = alloca [2 x [[S_FLOAT_TY]]],
+// ALL: [[VAR_PRIV:%.+]] = alloca [[S_FLOAT_TY]],
+// ALL: [[SIVAR_PRIV:%.+]] = alloca i{{[0-9]+}},
 // CHECK: store i{{[0-9]+}}* [[GTID_ADDR]], i{{[0-9]+}}** [[GTID_ADDR_REF:%.+]]
-// CHECK-NOT: [[T_VAR_PRIV]]
-// CHECK-NOT: [[VEC_PRIV]]
-// CHECK: {{.+}}:
+//

[clang] a881dc1 - Fix typo

2020-05-09 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2020-05-09T16:00:17-04:00
New Revision: a881dc1103579926f039e81c0d25626ff8a582a9

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

LOG: Fix typo

Added: 


Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index 1e164d3fe2b0..bc5c1682853b 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -8353,7 +8353,7 @@ class AMDGPUABIInfo final : public DefaultABIInfo {
 EltTys, (STy->getName() + ".coerce").str(), STy->isPacked());
   return llvm::StructType::get(getVMContext(), EltTys, STy->isPacked());
 }
-// Arrary types.
+// Array types.
 if (auto ATy = dyn_cast(Ty)) {
   auto T = ATy->getElementType();
   auto NT = coerceKernelArgumentType(T, FromAS, ToAS);



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

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

2020-05-09 Thread Fady Ghanim via Phabricator via cfe-commits

fghanim created this revision.
fghanim added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, guansong, hiraditya, 
yaxunl.
Herald added projects: clang, LLVM.

Added new methods to generate new runtime calls
Added all required defenitions
Added some target-specific types


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79675

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -93,6 +93,18 @@
 
 void OpenMPIRBuilder::initialize() { initializeTypes(M); }
 
+void OpenMPIRBuilder::setIntPtrTy(IntegerType *IntPtrTy) {
+  initializeIntPtrTy(IntPtrTy);
+}
+
+void OpenMPIRBuilder::setSizeTy(IntegerType *SizeTy) {
+  initializeSizeTy(SizeTy);
+}
+
+void OpenMPIRBuilder::setVoidPtrTy(PointerType *VoidPtrTy) {
+  initializeVoidPtrTy(VoidPtrTy);
+}
+
 void OpenMPIRBuilder::finalize() {
   for (OutlineInfo &OI : OutlineInfos) {
 assert(!OI.Blocks.empty() &&
@@ -576,9 +588,10 @@
  "Unexpected finalization stack state!");
 
   Instruction *PRegPreFiniTI = PRegPreFiniBB->getTerminator();
-  assert(PRegPreFiniTI->getNumSuccessors() == 1 &&
- PRegPreFiniTI->getSuccessor(0) == PRegExitBB &&
- "Unexpected CFG structure!");
+
+  //  assert(PRegPreFiniTI->getNumSuccessors() == 1 &&
+  // PRegPreFiniTI->getSuccessor(0) == PRegExitBB &&
+  // "Unexpected CFG structure!");
 
   InsertPointTy PreFiniIP(PRegPreFiniBB, PRegPreFiniTI->getIterator());
   FiniCB(PreFiniIP);
@@ -912,6 +925,93 @@
   ExitCall->getIterator());
 }
 
+OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::CreateCopyinClauseBlocks(
+InsertPointTy IP, Value *MasterAddr, Value *PrivateAddr,
+llvm::IntegerType *IntPtrTy, bool BranchtoEnd) {
+  if (!IP.isSet())
+return IP;
+
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+
+  BasicBlock *OMP_Entry = IP.getBlock();
+  Function *CurFn = OMP_Entry->getParent();
+  BasicBlock *CopyBegin =
+  BasicBlock::Create(M.getContext(), "copyin.not.master", CurFn);
+  BasicBlock *CopyEnd = nullptr;
+  // If entry block is terminated, split to preserve the branch to following
+  // basic block, otherwise, leave everything as is.
+  if (isa_and_nonnull(OMP_Entry->getTerminator())) {
+CopyEnd = OMP_Entry->splitBasicBlock(OMP_Entry->getTerminator(),
+ "copyin.not.master.end");
+OMP_Entry->getTerminator()->eraseFromParent();
+  } else {
+CopyEnd =
+BasicBlock::Create(M.getContext(), "copyin.not.master.end", CurFn);
+  }
+
+  Builder.SetInsertPoint(OMP_Entry);
+  Value *MasterPtr = Builder.CreatePtrToInt(MasterAddr, IntPtrTy);
+  Value *PrivatePtr = Builder.CreatePtrToInt(PrivateAddr, IntPtrTy);
+  Value *cmp = Builder.CreateICmpNE(MasterPtr, PrivatePtr);
+  Builder.CreateCondBr(cmp, CopyBegin, CopyEnd);
+
+  Builder.SetInsertPoint(CopyBegin);
+  if (BranchtoEnd) {
+BranchInst *br = Builder.CreateBr(CopyEnd);
+Builder.SetInsertPoint(br);
+  }
+
+  return Builder.saveIP();
+}
+
+CallInst *OpenMPIRBuilder::CreateOMPAlloc(const LocationDescription &Loc,
+  Value *Size, Value *Allocator,
+  std::string Name) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  Value *Args[] = {ThreadId, Size, Allocator};
+
+  Function *Fn = getOrCreateRuntimeFunction(OMPRTL___kmpc_alloc);
+
+  return Builder.CreateCall(Fn, Args, Name);
+}
+
+CallInst *OpenMPIRBuilder::CreateOMPFree(const LocationDescription &Loc,
+ Value *Addr, Value *Allocator,
+ std::string Name) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *ThreadId = getOrCreateThreadID(Ident);
+  Value *Args[] = {ThreadId, Addr, Allocator};
+  Function *Fn = getOrCreateRuntimeFunction(OMPRTL___kmpc_free);
+  return Builder.CreateCall(Fn, Args, Name);
+}
+
+CallInst *OpenMPIRBuilder::CreateCachedThreadPrivate(
+const LocationDescription &Loc, llvm::Value *Pointer,
+llvm::ConstantInt *Size, const llvm::Twine &Name) {
+  IRBuilder<>::InsertPointGuard IPG(Builder);
+  Builder.restoreIP(Loc.IP);
+
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocSt

[PATCH] D79676: [Clang][OpenMP][OMPBuilder] Moving OMP allocation and cache creation code to OMPBuilderCBHelpers

2020-05-09 Thread Fady Ghanim via Phabricator via cfe-commits

fghanim created this revision.
fghanim added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, guansong, yaxunl.
Herald added a project: clang.

Modified the OMPBuilderCBHelpers in the following ways:

- Moved location of class definition and deleted all constructors
- Moved OpenMP-specific address allocation of local variables
- Moved threadprivate variable creation for the current thread


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79676

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h

Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -26,6 +26,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/ABI.h"
 #include "clang/Basic/CapturedStmt.h"
@@ -77,6 +78,7 @@
 class ObjCAtSynchronizedStmt;
 class ObjCAutoreleasePoolStmt;
 class ReturnsNonNullAttr;
+class OMPExecutableDirective;
 
 namespace analyze_os_log {
 class OSLogBufferLayout;
@@ -256,114 +258,6 @@
 unsigned Index;
   };
 
-  // Helper class for the OpenMP IR Builder. Allows reusability of code used for
-  // region body, and finalization codegen callbacks. This will class will also
-  // contain privatization functions used by the privatization call backs
-  struct OMPBuilderCBHelpers {
-
-using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-
-/// Emit the Finalization for an OMP region
-/// \param CGF	The Codegen function this belongs to
-/// \param IP	Insertion point for generating the finalization code.
-static void FinalizeOMPRegion(CodeGenFunction &CGF, InsertPointTy IP) {
-  CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-  assert(IP.getBlock()->end() != IP.getPoint() &&
- "OpenMP IR Builder should cause terminated block!");
-
-  llvm::BasicBlock *IPBB = IP.getBlock();
-  llvm::BasicBlock *DestBB = IPBB->getUniqueSuccessor();
-  assert(DestBB && "Finalization block should have one successor!");
-
-  // erase and replace with cleanup branch.
-  IPBB->getTerminator()->eraseFromParent();
-  CGF.Builder.SetInsertPoint(IPBB);
-  CodeGenFunction::JumpDest Dest = CGF.getJumpDestInCurrentScope(DestBB);
-  CGF.EmitBranchThroughCleanup(Dest);
-}
-
-/// Emit the body of an OMP region
-/// \param CGF	The Codegen function this belongs to
-/// \param RegionBodyStmt	The body statement for the OpenMP region being
-/// 			 generated
-/// \param CodeGenIP	Insertion point for generating the body code.
-/// \param FiniBB	The finalization basic block
-static void EmitOMPRegionBody(CodeGenFunction &CGF,
-  const Stmt *RegionBodyStmt,
-  InsertPointTy CodeGenIP,
-  llvm::BasicBlock &FiniBB) {
-  llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
-  if (llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator())
-CodeGenIPBBTI->eraseFromParent();
-
-  CGF.Builder.SetInsertPoint(CodeGenIPBB);
-
-  CGF.EmitStmt(RegionBodyStmt);
-
-  if (CGF.Builder.saveIP().isSet())
-CGF.Builder.CreateBr(&FiniBB);
-}
-
-/// RAII for preserving necessary info during Outlined region body codegen.
-class OutlinedRegionBodyRAII {
-
-  llvm::AssertingVH OldAllocaIP;
-  CodeGenFunction::JumpDest OldReturnBlock;
-  CodeGenFunction &CGF;
-
-public:
-  OutlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
- llvm::BasicBlock &RetBB)
-  : CGF(cgf) {
-assert(AllocaIP.isSet() &&
-   "Must specify Insertion point for allocas of outlined function");
-OldAllocaIP = CGF.AllocaInsertPt;
-CGF.AllocaInsertPt = &*AllocaIP.getPoint();
-
-OldReturnBlock = CGF.ReturnBlock;
-CGF.ReturnBlock = CGF.getJumpDestInCurrentScope(&RetBB);
-  }
-
-  ~OutlinedRegionBodyRAII() {
-CGF.AllocaInsertPt = OldAllocaIP;
-CGF.ReturnBlock = OldReturnBlock;
-  }
-};
-
-/// RAII for preserving necessary info during inlined region body codegen.
-class InlinedRegionBodyRAII {
-
-  llvm::AssertingVH OldAllocaIP;
-  CodeGenFunction &CGF;
-
-public:
-  InlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
-llvm::BasicBlock &FiniBB)
-  : CGF(cgf) {
-// Alloca insertion block should be in the entry block of the containing
-// function so it expects an empty AllocaIP in which case will reuse the
-// old alloca insertion point, or a new AllocaIP in the same block as
-// the old one
-assert((!AllocaIP.is

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

> Do you object to say that the call site argument and the argument point to 
> distinct memory locations or something else?

Like I said, my issue is with the "Attributes on the call site argument and 
function argument are associated with the original and copied memory 
respectively".  I assume this means "attributes other than byval".  If I'm 
understanding this correctly, this means it isn't legal to copy attributes from 
the caller to the callee.  If an argument is marked "readnone byval" on a 
function, it's illegal to copy that "readnone" to the callsite, because the 
readnone would then be associated with the original memory, not the copied 
memory.  Or, a more silly example, say you had "byval returned" on the called 
function, and that got copied to the callsite: that clearly can't mean the 
original pointer is returned by the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

2020-05-09 Thread Nathan James via Phabricator via cfe-commits

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Methods that override virtual methods will now get renamed if the initial 
virtual method has a name violation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79674

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -267,14 +267,32 @@
   virtual ~AOverridden() = default;
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual 
method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
 };
 
 class COverriding : public AOverridden {
 public:
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  
+  void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method();
+this->BadBaseMethod();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method();
+AOverridden::BadBaseMethod();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
+COverriding::BadBaseMethod();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+  }
 };
 
+void VirtualCall(AOverridden &a_vItem) {
+  a_vItem.BadBaseMethod();
+  // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+}
+
 template 
 class CRTPBase {
 public:
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -127,6 +127,26 @@
  this));
 }
 
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+  if (Method->size_overridden_methods() != 1)
+return nullptr; // no overrides
+  while (true) {
+const CXXMethodDecl *Next = *Method->begin_overridden_methods();
+assert(Next && "Overridden method shouldn't be null");
+if (Next->size_overridden_methods() == 0) {
+  return Next;
+}
+if (Next->size_overridden_methods() == 1) {
+  Method = Next;
+  continue;
+}
+return nullptr; // Multiple overrides
+  }
+}
+
 void RenamerClangTidyCheck::addUsage(
 const RenamerClangTidyCheck::NamingCheckId &Decl, SourceRange Range,
 SourceManager *SourceMgr) {
@@ -160,7 +180,10 @@
 
 void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
  SourceManager *SourceMgr) {
-
+  if (const auto *Method = dyn_cast(Decl)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+  Decl = Overridden;
+  }
   return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),

Decl->getNameAsString()),
   Range, SourceMgr);
@@ -390,6 +413,14 @@
   }
 }
 
+// Fix overridden methods
+if (const auto *Method = Result.Nodes.getNodeAs("decl")) {
+  if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+addUsage(Overridden, Method->getLocation());
+return; // Don't try to add the actual decl as a Failure.
+  }
+}
+
 // Ignore ClassTemplateSpecializationDecl which are creating duplicate
 // replacements with CXXRecordDecl.
 if (isa(Decl))


Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -267,14 +267,32 @@
   virtual ~AOverridden() = default;
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
 };
 
 class COverriding : public AOverridden {
 public:
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  
+  void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert updated this revision to Diff 263026.
jdoerfert added a comment.

Minor wording tweak


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636

Files:
  llvm/docs/LangRef.rst


Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1043,10 +1043,18 @@
 is unable to modify the value in the caller. This attribute is only
 valid on LLVM pointer arguments. It is generally used to pass
 structs and arrays by value, but is also valid on pointers to
-scalars. The copy is considered to belong to the caller not the
-callee (for example, ``readonly`` functions should not write to
-``byval`` parameters). This is not a valid attribute for return
-values.
+scalars. The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memory and
+the pointer argument in the callee is referring to the copy. Attributes on
+the call site argument and function argument are associated with the
+original and copied memory respectively. The copy is considered to be local
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, which is the call site argument, is always read. This
+effect is also associated with the call site, thus it cannot be `readnone`
+or `writeonly` regardless of the callee. This is not a valid attribute for
+return values.
 
 The byval attribute also supports an optional type argument, which must be
 the same as the pointee type of the argument.


Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1043,10 +1043,18 @@
 is unable to modify the value in the caller. This attribute is only
 valid on LLVM pointer arguments. It is generally used to pass
 structs and arrays by value, but is also valid on pointers to
-scalars. The copy is considered to belong to the caller not the
-callee (for example, ``readonly`` functions should not write to
-``byval`` parameters). This is not a valid attribute for return
-values.
+scalars. The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memory and
+the pointer argument in the callee is referring to the copy. Attributes on
+the call site argument and function argument are associated with the
+original and copied memory respectively. The copy is considered to be local
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, which is the call site argument, is always read. This
+effect is also associated with the call site, thus it cannot be `readnone`
+or `writeonly` regardless of the callee. This is not a valid attribute for
+return values.
 
 The byval attribute also supports an optional type argument, which must be
 the same as the pointee type of the argument.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2020-05-09 Thread Bhopesh Bassi via Phabricator via cfe-commits

bbassi added a comment.

@MyDeveloperDay Thanks. This would be my first revision and I have few 
questions before I start coding. Would you be able to answer those over email? 
They are mainly about the design of clang-format and some existing options.


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

https://reviews.llvm.org/D33029



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

[clang-tools-extra] 0e49ac7 - [NFC] Small rework to RenamerClangTidyCheck addUsage

2020-05-09 Thread Nathan James via cfe-commits


Author: Nathan James
Date: 2020-05-09T18:57:05+01:00
New Revision: 0e49ac73eaf554ad4135f51b03ea4eadaebf0466

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

LOG: [NFC] Small rework to RenamerClangTidyCheck addUsage

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index d8bdbcc8c25e..56a4c08b7cbc 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -127,9 +127,9 @@ void RenamerClangTidyCheck::registerPPCallbacks(
  this));
 }
 
-static void addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures,
- const RenamerClangTidyCheck::NamingCheckId &Decl,
- SourceRange Range, SourceManager *SourceMgr = nullptr) {
+void RenamerClangTidyCheck::addUsage(
+const RenamerClangTidyCheck::NamingCheckId &Decl, SourceRange Range,
+SourceManager *SourceMgr) {
   // Do nothing if the provided range is invalid.
   if (Range.isInvalid())
 return;
@@ -146,7 +146,8 @@ static void 
addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures,
 
   // Try to insert the identifier location in the Usages map, and bail out if 
it
   // is already in there
-  RenamerClangTidyCheck::NamingCheckFailure &Failure = Failures[Decl];
+  RenamerClangTidyCheck::NamingCheckFailure &Failure =
+  NamingCheckFailures[Decl];
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
 return;
 
@@ -157,12 +158,10 @@ static void 
addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures,
 Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro;
 }
 
-/// Convenience method when the usage to be added is a NamedDecl
-static void addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range,
- SourceManager *SourceMgr = nullptr) {
-  return addUsage(Failures,
-  RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
+void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
+ SourceManager *SourceMgr) {
+
+  return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),

Decl->getNameAsString()),
   Range, SourceMgr);
 }
@@ -243,15 +242,13 @@ void RenamerClangTidyCheck::check(const 
MatchFinder::MatchResult &Result) {
   if (const auto *Decl =
   Result.Nodes.getNodeAs("classRef")) {
 
-addUsage(NamingCheckFailures, Decl->getParent(),
- Decl->getNameInfo().getSourceRange());
+addUsage(Decl->getParent(), Decl->getNameInfo().getSourceRange());
 
 for (const auto *Init : Decl->inits()) {
   if (!Init->isWritten() || Init->isInClassMemberInitializer())
 continue;
   if (const FieldDecl *FD = Init->getAnyMember())
-addUsage(NamingCheckFailures, FD,
- SourceRange(Init->getMemberLocation()));
+addUsage(FD, SourceRange(Init->getMemberLocation()));
   // Note: delegating constructors and base class initializers are handled
   // via the "typeLoc" matcher.
 }
@@ -268,7 +265,7 @@ void RenamerClangTidyCheck::check(const 
MatchFinder::MatchResult &Result) {
 // we want instead to replace the next token, that will be the identifier.
 Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd());
 
-addUsage(NamingCheckFailures, Decl->getParent(), Range);
+addUsage(Decl->getParent(), Range);
 return;
   }
 
@@ -286,7 +283,7 @@ void RenamerClangTidyCheck::check(const 
MatchFinder::MatchResult &Result) {
 // further TypeLocs handled below
 
 if (Decl) {
-  addUsage(NamingCheckFailures, Decl, Loc->getSourceRange());
+  addUsage(Decl, Loc->getSourceRange());
   return;
 }
 
@@ -297,7 +294,7 @@ void RenamerClangTidyCheck::check(const 
MatchFinder::MatchResult &Result) {
   SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc());
   if (const auto *ClassDecl = dyn_cast(Decl)) {
 if (const NamedDecl *TemplDecl = ClassDecl->getTemplatedDecl())
-  addUsage(NamingCheckFailures, TemplDecl, Range);
+  addUsage(TemplDecl, Range);
 return;
   }
 }
@@ -305,7 +302,7 @@ void RenamerClangTidyCheck::check(const 
MatchFinder::MatchResult &Result) {
 if (const auto &Ref =
 Loc->getAs()) {
   if (const TagD

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert updated this revision to Diff 263024.
jdoerfert marked 3 inline comments as done.
jdoerfert added a comment.

Fix spelling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636

Files:
  llvm/docs/LangRef.rst


Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1043,10 +1043,18 @@
 is unable to modify the value in the caller. This attribute is only
 valid on LLVM pointer arguments. It is generally used to pass
 structs and arrays by value, but is also valid on pointers to
-scalars. The copy is considered to belong to the caller not the
-callee (for example, ``readonly`` functions should not write to
-``byval`` parameters). This is not a valid attribute for return
-values.
+scalars. The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memory and
+the pointer argument in the callee is referring to the copy. Attributes on
+the call site argument and function argument are associated with the
+original and copied memory respectively. The copy is considered to be local
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, thus the call site argument, is always read. This
+effect is also associated with the call site, thus it cannot be `readnone`
+or `writeonly` regardless of the callee. This is not a valid attribute for
+return values.
 
 The byval attribute also supports an optional type argument, which must be
 the same as the pointee type of the argument.


Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1043,10 +1043,18 @@
 is unable to modify the value in the caller. This attribute is only
 valid on LLVM pointer arguments. It is generally used to pass
 structs and arrays by value, but is also valid on pointers to
-scalars. The copy is considered to belong to the caller not the
-callee (for example, ``readonly`` functions should not write to
-``byval`` parameters). This is not a valid attribute for return
-values.
+scalars. The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memory and
+the pointer argument in the callee is referring to the copy. Attributes on
+the call site argument and function argument are associated with the
+original and copied memory respectively. The copy is considered to be local
+memory of the callee. That means, a callee can write a ``byval`` parameter
+and still be ``readonly`` or ``readnone`` because the write is, similar to
+a write to an ``alloca``, not visible from the outside. To create the copy
+the original memory, thus the call site argument, is always read. This
+effect is also associated with the call site, thus it cannot be `readnone`
+or `writeonly` regardless of the callee. This is not a valid attribute for
+return values.
 
 The byval attribute also supports an optional type argument, which must be
 the same as the pointee type of the argument.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

> "Attributes on the call site argument and function argument are associated 
> with the original and copied memory respectively"
> 
> This seems to fly in the face of existing practice, which is that function 
> attributes are copied to each callsite. I'd strongly prefer to keep the 
> meaning of the attributes consistent, even if it leads to a weird result like 
> writing to an argument marked "readonly".

First, this doesn't break with the practice that argument attributes are 
copied/applied to each call site, `byval` is still present at both the call 
site argument and the argument. It does state however that the pointer of the 
attribute is pointing to something different for the call site argument and for 
the argument. That is no different to the current semantic, as far as I can 
tell, just spelled out. Do you object to say that the call site argument and 
the argument point to distinct memory locations or something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[clang-tools-extra] c746781 - [clangd] Fix data race in BackgroundIndex test

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


Author: Kadir Cetinkaya
Date: 2020-05-09T18:15:27+02:00
New Revision: c746781f5085a965cfc64bd8ddf904217b797ab8

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

LOG: [clangd] Fix data race in BackgroundIndex test

MockFSProvider is not thread-safe. Make sure we don't modify it while
background index is working.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index 934f1abb1a7c..b870a8ed5a83 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -443,6 +443,7 @@ TEST_F(BackgroundIndexTest, NoDotsInAbsPath) {
   OverlayCDB CDB(/*Base=*/nullptr);
   BackgroundIndex Idx(Context::empty(), FS, CDB,
   [&](llvm::StringRef) { return &MSS; });
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
 
   tooling::CompileCommand Cmd;
   FS.Files[testPath("root/A.cc")] = "";
@@ -450,14 +451,15 @@ TEST_F(BackgroundIndexTest, NoDotsInAbsPath) {
   Cmd.Directory = testPath("root/build");
   Cmd.CommandLine = {"clang++", "../A.cc"};
   CDB.setCompileCommand(testPath("root/build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
 
   FS.Files[testPath("root/B.cc")] = "";
   Cmd.Filename = "./B.cc";
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "./B.cc"};
   CDB.setCompileCommand(testPath("root/./B.cc"), Cmd);
-
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
   for (llvm::StringRef AbsPath : MSS.AccessedPaths.keys()) {
 EXPECT_FALSE(AbsPath.contains("./")) << AbsPath;
 EXPECT_FALSE(AbsPath.contains("../")) << AbsPath;



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

[PATCH] D79673: Allow 32-bit pointer extensions to be used without -fms-extensions

2020-05-09 Thread David Zarzycki via Phabricator via cfe-commits

davezarzycki added a comment.

In D79673#2028204 , @rnk wrote:

> Needs a test.
>
> I believe these are only implemented for x86 in LLVM. What happens if you try 
> to use this on non-x86? I wouldn't be surprised if we crash, but we should 
> probably produce a proper error and test it.

I just tested top-of-tree `clang -fms-extensions -target arm64-unknown-linux` 
(sans this patch) and apparently the attributes are ignored, which could cause 
crashes at run time if a later compiler actually honors this attributes on 
non-x86 targets and people try to mix object files created by older and newer 
compilers.

I agree that the bugs you point out are worth fixing. That being said, I'm not 
looking to fix all of the bugs with these attributes with this patch. If it's 
okay, I'd like to focus on exposing the feature outside of `-fms-extensions`. 
If people want to ignore the Microsoft extension and design how this feature 
"should" work, that's fine too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79673

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

[PATCH] D79631: #pragma float_control should be permitted at namespace scope

2020-05-09 Thread Melanie Blower via Phabricator via cfe-commits

mibintc updated this revision to Diff 263020.
mibintc added a comment.

I corrected the assertion error by propagating usesFPIntrin flag from the 
template function to the template instantiation and added a test case. Ready 
for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79631

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/Parser/fp-floatcontrol-syntax.cpp

Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -10,10 +10,10 @@
 #pragma float_control(pop)
 #pragma float_control(precise, on, push)
 void check_stack() {
-#pragma float_control(push)   // expected-error {{can only appear at file scope}}
-#pragma float_control(pop)// expected-error {{can only appear at file scope}}
-#pragma float_control(precise, on, push)  // expected-error {{can only appear at file scope}}
-#pragma float_control(except, on, push)   // expected-error {{can only appear at file scope}}
+#pragma float_control(push)   // expected-error {{can only appear at file scope or namespace scope}}
+#pragma float_control(pop)// expected-error {{can only appear at file scope or namespace scope}}
+#pragma float_control(precise, on, push)  // expected-error {{can only appear at file scope or namespace scope}}
+#pragma float_control(except, on, push)   // expected-error {{can only appear at file scope or namespace scope}}
 #pragma float_control(except, on, push, junk) // expected-error {{float_control is malformed}}
   return;
 }
Index: clang/test/CodeGen/fp-floatcontrol-pragma.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-pragma.cpp
+++ clang/test/CodeGen/fp-floatcontrol-pragma.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -DEXCEPT=1 -fcxx-exceptions -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-NS %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -verify -DFENV_ON=1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
 
@@ -59,3 +60,56 @@
   z = z * z;
 //CHECK: = fmul float
 }
+
+#if EXCEPT
+namespace ns {
+// Check that pragma float_control can appear in namespace.
+#pragma float_control(except, on, push)
+float exc_on(double x, float zero) {
+// CHECK-NS: define {{.*}}exc_on{{.*}}
+  {} try {
+x = 1.0 / zero; /* division by zero, the result unused */
+//CHECK-NS: llvm.experimental.constrained.fdiv{{.*}}
+  } catch (...) {}
+  return zero;
+}
+}
+
+// Check pragma is still effective after namespace closes
+float exc_still_on(double x, float zero) {
+// CHECK-NS: define {{.*}}exc_still_on{{.*}}
+  {} try {
+x = 1.0 / zero; /* division by zero, the result unused */
+//CHECK-NS: llvm.experimental.constrained.fdiv{{.*}}
+  } catch (...) {}
+  return zero;
+}
+
+#pragma float_control(pop)
+float exc_off(double x, float zero) {
+// CHECK-NS: define {{.*}}exc_off{{.*}}
+  {} try {
+x = 1.0 / zero; /* division by zero, the result unused */
+//CHECK-NS: fdiv contract double
+  } catch (...) {}
+  return zero;
+}
+
+namespace fc_template_namespace {
+#pragma float_control(except, on, push)
+template 
+T exc_on(double x, T zero) {
+// CHECK-NS: define {{.*}}fc_template_namespace{{.*}}
+  {} try {
+x = 1.0 / zero; /* division by zero, the result unused */
+//CHECK-NS: llvm.experimental.constrained.fdiv{{.*}}
+  } catch (...) {}
+  return zero;
+}
+}
+
+#pragma float_control(pop)
+float xx(double x, float z) {
+  return fc_template_namespace::exc_on(x, z);
+}
+#endif // EXCEPT
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1911,6 +1911,7 @@
 D->hasWrittenPrototype(), D->getConstexprKind(),
 TrailingRequiresClause);
 Function->setRangeEnd(D->getSourceRange().getEnd());
+Function->setUsesFPIntrin(D->usesFPIntrin());
   }
 
   if (D->isInlined())
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -413,8 +413,8 @@
   auto NewValue = FpPragmaStack.CurrentValue;
   FPOptions NewFPFeatures(NewValue);
   if ((Action == PSK_Push_Set || Action == PSK_Push || Action == PSK_Pop) &&
-  !CurContext->isTranslationUnit()) {
-// Push and pop can only occur at file scope.
+  !(CurContext->isTranslationUnit()) && !CurContext->isNamespace()) {
+// Push and pop can only occur at file or

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

2020-05-09 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

Some copy editing comments, but I agree with the semantics: From the IR 
perspective, it is better to think of argument stack memory as belonging to the 
callee. A byval argument has more in common with a local static alloca than a 
passed in pointer.




Comment at: llvm/docs/LangRef.rst:1046
 structs and arrays by value, but is also valid on pointers to
-scalars. The copy is considered to belong to the caller not the
-callee (for example, ``readonly`` functions should not write to
-``byval`` parameters). This is not a valid attribute for return
-values.
+scalars The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memmory and

Missing period? "scalars The copy"



Comment at: llvm/docs/LangRef.rst:1047
+scalars The copy is conceptually made on the call edge. This means that
+the pointer argument at the call site refers to the original memmory and
+the pointer argument in the callee is refering to the copy. Attributes on

typo "memmory"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636



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

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-09 Thread Nathan James via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82ddae061b4b: [clang-tidy] RenamerClangTidy now renames 
dependent member expr when the member… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73052

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
@@ -1,7 +1,9 @@
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase} \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack}, \
+// RUN: {key: readability-identifier-naming.AggressiveDependentMemberLookup, value: 1} \
 // RUN:  ]}' -- -fno-delayed-template-parsing
 
 int set_up(int);
@@ -63,32 +65,23 @@
   // CHECK-FIXES: {{^}}  int getBar2() const { return this->BarBaz; } // comment-9
 };
 
-TempTest x; //force an instantiation
-
-int blah() {
-  return x.getBar2(); // gotta have a reference to the getBar2 so that the
-  // compiler will generate the function and resolve
-  // the DependantScopeMemberExpr
-}
-
 namespace Bug41122 {
 namespace std {
 
 // for this example we aren't bothered about how std::vector is treated
-template  //NOLINT
-class vector { //NOLINT
-public:
-  void push_back(bool); //NOLINT
-  void pop_back(); //NOLINT
-}; //NOLINT
-}; // namespace std
+template// NOLINT
+struct vector { // NOLINT
+  void push_back(bool); // NOLINT
+  void pop_back();  // NOLINT
+};  // NOLINT
+};  // namespace std
 
-class Foo { 
+class Foo {
   std::vector &stack;
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming]
 public:
   Foo(std::vector &stack)
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
   // CHECK-FIXES: {{^}}  Foo(std::vector &Stack)
   : stack(stack) {
 // CHECK-FIXES: {{^}}  : Stack(Stack) {
@@ -134,4 +127,92 @@
 void foo() {
   Container container;
 }
-}; // namespace CtorInits
+} // namespace CtorInits
+
+namespace resolved_dependance {
+template 
+struct A0 {
+  int value;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
+  A0 &operator=(const A0 &Other) {
+value = Other.value;   // A0
+this->value = Other.value; // A0
+// CHECK-FIXES:  {{^}}Value = Other.Value;   // A0
+// CHECK-FIXES-NEXT: {{^}}this->Value = Other.Value; // A0
+return *this;
+  }
+  void outOfLineReset();
+};
+
+template 
+void A0::outOfLineReset() {
+  this->value -= value; // A0
+  // CHECK-FIXES: {{^}}  this->Value -= Value; // A0
+}
+
+template 
+struct A1 {
+  int value; // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
+  // CHECK-FIXES: {{^}}  int Value; // A1
+  int GetValue() const { return value; } // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for method 'GetValue'
+  // CHECK-FIXES {{^}}  int getValue() const { return Value; } // A1
+  void SetValue(int Value) { this->value = Value; } // A1
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'SetValue'
+  // CHECK-FIXES {{^}}  void setValue(int Value) { this->Value = Value; } // A1
+  A1 &operator=(const A1 &Other) {
+this->SetValue(Other.GetValue()); // A1
+this->value = Other.value;// A1
+// CHECK-FIXES:  {{^}}this->setValue(Other.getValue()); // A1
+// CHECK-FIXES-NEXT: {{^}}this->Value = Other.Value;// A1
+return *this;
+  }
+  void outOfLineReset();
+};
+
+template 
+void A1::outOfLineReset() {
+  this->value -= value; // A1
+  // CHECK-FIXES: {{^}}  this->Value

[PATCH] D79673: Allow 32-bit pointer extensions to be used without -fms-extensions

2020-05-09 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

Needs a test.

I believe these are only implemented for x86 in LLVM. What happens if you try 
to use this on non-x86? I wouldn't be surprised if we crash, but we should 
probably produce a proper error and test it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79673



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

[clang-tools-extra] 82ddae0 - [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-09 Thread Nathan James via cfe-commits


Author: Nathan James
Date: 2020-05-09T16:21:49+01:00
New Revision: 82ddae061b4ba11895756004d559160cbd519fff

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

LOG: [clang-tidy] RenamerClangTidy now renames dependent member expr when the 
member can be resolved

Summary:
Sometimes in templated code Member references are reported as 
`DependentScopeMemberExpr` because that's what the standard dictates, however 
in many trivial cases it is easy to resolve the reference to its actual Member.
Take this code:
```
template
class A{
  int value;
  A& operator=(const A& Other){
value = Other.value;
this->value = Other.value;
return *this;
  }
};
```
When ran with `clang-tidy file.cpp -checks=readability-identifier-naming 
--config="{CheckOptions: [{key: readability-identifier-naming.MemberPrefix, 
value: m_}]}" -fix`
Current behaviour:
```
template
class A{
  int m_value;
  A& operator=(const A& Other){
m_value = Other.value;
this->value = Other.value;
return *this;
  }
};
```
As `this->value` and `Other.value` are Dependent they are ignored when creating 
the fix-its, however this can easily be resolved.
Proposed behaviour:
```
template
class A{
  int m_value;
  A& operator=(const A& Other){
m_value = Other.m_value;
this->m_value = Other.m_value;
return *this;
  }
};
```

Reviewers: aaron.ballman, JonasToth, alexfh, hokein, gribozavr2

Reviewed By: aaron.ballman

Subscribers: merge_guards_bot, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index d562d8c3d213..9619524a0c03 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -47,6 +47,7 @@ ReservedIdentifierCheck::ReservedIdentifierCheck(StringRef 
Name,
   Options.get("AllowedIdentifiers", ""))) {}
 
 void ReservedIdentifierCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  RenamerClangTidyCheck::storeOptions(Opts);
   Options.store(Opts, "Invert", Invert);
   Options.store(Opts, "AllowedIdentifiers",
 utils::options::serializeStringList(AllowedIdentifiers));

diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 9146e7356c9c..7c7de74a0c9e 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -170,6 +170,7 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
 
 void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  RenamerClangTidyCheck::storeOptions(Opts);
   for (size_t i = 0; i < SK_Count; ++i) {
 if (NamingStyles[i]) {
   if (NamingStyles[i]->Case) {

diff  --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 3523cf5dcf16..d8bdbcc8c25e 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -14,8 +14,7 @@
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/DenseMapInfo.h"
-#include "llvm/Support/Debug.h"
-#include "llvm/Support/Format.h"
+#include "llvm/ADT/PointerIntPair.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
@@ -93,9 +92,16 @@ class RenamerClangTidyCheckPPCallbacks : public PPCallbacks {
 
 RenamerClangTidyCheck::RenamerClangTidyCheck(StringRef CheckName,
  ClangTidyContext *Context)
-: ClangTidyCheck(CheckName, Context) {}
+: ClangTidyCheck(CheckName, Context),
+  AggressiveDependentMemberLookup(
+  Options.getLocalOrGlobal("AggressiveDependentMemberLookup", false)) 
{}
 RenamerClangTidyCheck::~RenamerClangTidyCheck() = default;
 
+void RenamerClangTidyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AggressiveDependentMemberL

[PATCH] D79673: Allow 32-bit pointer extensions to be used without -fms-extensions

2020-05-09 Thread David Zarzycki via Phabricator via cfe-commits

davezarzycki created this revision.
davezarzycki added reviewers: akhuang, rnk, rsmith.
davezarzycki added a project: clang.
Herald added a reviewer: aaron.ballman.

One should not need to use `-fms-extensions` in order to use 32-bit pointers on 
64-bit platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79673

Files:
  clang/include/clang/Basic/Attr.td


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3080,22 +3080,22 @@
 }
 
 def Ptr32 : TypeAttr {
-  let Spellings = [Keyword<"__ptr32">];
+  let Spellings = [Keyword<"__ptr32">, Clang<"ptr32">];
   let Documentation = [Ptr32Docs];
 }
 
 def Ptr64 : TypeAttr {
-  let Spellings = [Keyword<"__ptr64">];
+  let Spellings = [Keyword<"__ptr64">, Clang<"ptr64">];
   let Documentation = [Ptr64Docs];
 }
 
 def SPtr : TypeAttr {
-  let Spellings = [Keyword<"__sptr">];
+  let Spellings = [Keyword<"__sptr">, Clang<"sptr">];
   let Documentation = [SPtrDocs];
 }
 
 def UPtr : TypeAttr {
-  let Spellings = [Keyword<"__uptr">];
+  let Spellings = [Keyword<"__uptr">, Clang<"uptr">];
   let Documentation = [UPtrDocs];
 }
 


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3080,22 +3080,22 @@
 }
 
 def Ptr32 : TypeAttr {
-  let Spellings = [Keyword<"__ptr32">];
+  let Spellings = [Keyword<"__ptr32">, Clang<"ptr32">];
   let Documentation = [Ptr32Docs];
 }
 
 def Ptr64 : TypeAttr {
-  let Spellings = [Keyword<"__ptr64">];
+  let Spellings = [Keyword<"__ptr64">, Clang<"ptr64">];
   let Documentation = [Ptr64Docs];
 }
 
 def SPtr : TypeAttr {
-  let Spellings = [Keyword<"__sptr">];
+  let Spellings = [Keyword<"__sptr">, Clang<"sptr">];
   let Documentation = [SPtrDocs];
 }
 
 def UPtr : TypeAttr {
-  let Spellings = [Keyword<"__uptr">];
+  let Spellings = [Keyword<"__uptr">, Clang<"uptr">];
   let Documentation = [UPtrDocs];
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77249: [MSan] Pass command line options to MSan with new pass manager

2020-05-09 Thread Vitaly Buka via Phabricator via cfe-commits

vitalybuka added inline comments.



Comment at: compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp:4
+// this test.
+// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 \
+// RUN: -fexperimental-new-pass-manager -O3 %s -o %t && \

vitalybuka wrote:
> rnk wrote:
> > leonardchan wrote:
> > > nemanjai wrote:
> > > > nemanjai wrote:
> > > > > vitalybuka wrote:
> > > > > > Why not to add RUN: section with -fexperimental-new-pass-manager 
> > > > > > into original tests?
> > > > > I just felt that this is a simpler way forward for a couple of 
> > > > > reasons:
> > > > > 1. Once the default switches, it is a very obvious change to just 
> > > > > delete these files rather than digging through the code inside the 
> > > > > existing ones
> > > > > 2. Many of the tests actually contain the testing that is split up 
> > > > > into multiple steps so I would have to duplicate all the steps for 
> > > > > the NPM vs. default builds:
> > > > > - compile/link
> > > > > - run with one option set and FileCheck
> > > > > - run with another option set and FileCheck
> > > > > - rinse/repeat
> > > > > (example: chained_origin_limits.cpp)
> > > > > 
> > > > > But of course, if there are strong objections to this approach, I can 
> > > > > certainly go the other way.
> > > > Seems Phabricator reformatted what I wrote here. Points 3, 4, 5, 6 were 
> > > > supposed to be sub-bullets for 2.
> > > > Basically, I tried to describe that in the mentioned test case, I would 
> > > > have to replicate a number of subsequent steps for each `RUN` directive 
> > > > that invokes the compiler.
> > > If we're going this way, I think the original tests should explicitly 
> > > have `-fno-experimental-new-pass-manager`. Also no strong preference 
> > > towards either way.
> > I don't think we should even make changes to the tests in compiler-rt. We 
> > should write a targeted test in clang/test/CodeGen that ensures these 
> > options are passed down correctly to the MSan instrumentation pass. It 
> > should be easy to FileCheck the IR to look for the appropriate 
> > instrumentation callbacks. We can run that test with the new and old PM.
> I also prefer not have these new tests.
> Maybe we can add some option to run all compiler-rt tests with new PM. It 
> should be a separate patch.
maybe COMPILER_RT_TEST_COMPILER_CFLAGS is enough


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77249



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

[clang] 0b97833 - LTO.h - reduce includes to forward declarations. NFC.

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


Author: Simon Pilgrim
Date: 2020-05-09T15:10:51+01:00
New Revision: 0b9783350b3a9644dc6e0ba94c0f6a87ca45cb36

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

LOG: LTO.h - reduce includes to forward declarations. NFC.

Add missing ToolOutputFile.h dependency to BackendUtil.cpp

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/LTO/LTO.h

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index ba61a93a4d1a..efbc775323f5 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
+#include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"

diff  --git a/llvm/include/llvm/LTO/LTO.h b/llvm/include/llvm/LTO/LTO.h
index 0a635b45e5a2..c25aa077304b 100644
--- a/llvm/include/llvm/LTO/LTO.h
+++ b/llvm/include/llvm/LTO/LTO.h
@@ -17,28 +17,24 @@
 
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringSet.h"
-#include "llvm/IR/DiagnosticInfo.h"
-#include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/LTO/Config.h"
-#include "llvm/Linker/IRMover.h"
 #include "llvm/Object/IRSymtab.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/thread.h"
-#include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 
 namespace llvm {
 
 class BitcodeModule;
 class Error;
+class IRMover;
 class LLVMContext;
 class MemoryBufferRef;
 class Module;
-class Target;
 class raw_pwrite_stream;
+class Target;
+class ToolOutputFile;
 
 /// Resolve linkage for prevailing symbols in the \p Index. Linkage changes
 /// recorded in the index and the ThinLTO backends must apply the changes to



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

[clang-tools-extra] 84cbd47 - [clangd] Fix a data race in RecordsLatencies test

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


Author: Kadir Cetinkaya
Date: 2020-05-09T15:42:21+02:00
New Revision: 84cbd472e59236bd8ec541bc764ababc6a10a878

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

LOG: [clangd] Fix a data race in RecordsLatencies test

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp 
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index c60b264baa9c..fe752821859d 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -156,7 +156,7 @@ TEST_F(LSPTest, RecordsLatencies) {
   llvm::StringLiteral MethodName = "method_name";
   EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), 
testing::SizeIs(0));
   llvm::consumeError(Client.call(MethodName, {}).take().takeError());
-  Client.sync();
+  stop();
   EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), 
testing::SizeIs(1));
 }
 } // namespace



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

[PATCH] D79629: [Clang][Driver]Pass LLVM options to lld in case of LTO

2020-05-09 Thread bin via Phabricator via cfe-commits

bin.narwal added a comment.

In D79629#2026974 , @MaskRay wrote:

> `-mllvm,foobar` is for compilation (.c/.cc -> .o)
>
> `-Wl,-mllvm,foobar` for LTO options. For linking, `-mllvm,foobar` is not used 
> and thus warned.


Right, that's reasonable design.  The wrong UseLLD condition is a typo, anyway, 
I will discard this patch.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79629



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

[PATCH] D79669: [clangd] Filter pch related flags coming from the user

2020-05-09 Thread Sam McCall via Phabricator via cfe-commits

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79669



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

[PATCH] D79465: [clang-format] Fix line lengths w/ comments in align

2020-05-09 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

I've been able to reproduce and the patch looks good, I've just not had a 
chance to read the review in-depth (I don't know this section of the code as 
well as other parts).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79465



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2020-05-09 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D33029#1976981 , @bbassi wrote:

> @MyDeveloperDay  hey, I am currently working on this, and adding a new option 
> called BreakBeforeClosingBracket. I have some questions to understand the 
> existing code, they might not be directly linked to this change so I am not 
> sure if this the best place to ask those questions. What do you think? e.g. I 
> don't understand what FakeLParens is and have some questions about that.

@bbassi, you might want to consider starting a new revision if you are going to 
add a different option. Its probably best to discuss questions about adding 
that option in its own revision rather than confusing the conversation here. 
(you can always cross reference with a mention.

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

https://reviews.llvm.org/D33029

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

[PATCH] D79669: [clangd] Filter pch related flags coming from the user

2020-05-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov, mgorny.
Herald added a project: clang.

PCH format is unstable, hence using a preamble built with a different
version of clang (or even worse, a different compiler) might result in
unexpected behaviour.

PCH creation on the other hand is something clangd wouldn't want to perform, as
it doesn't generate any output files.

This patch makes sure clangd drops any PCH related compile commands after
parsing the command line args.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79669

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CompilerTests.cpp

Index: clang-tools-extra/clangd/unittests/CompilerTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CompilerTests.cpp
@@ -0,0 +1,55 @@
+//===-- CompilerTests.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Compiler.h"
+#include "TestTU.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::IsEmpty;
+
+TEST(BuildCompilerInvocation, DropsPCH) {
+  IgnoreDiagnostics Diags;
+  TestTU TU;
+  TU.AdditionalFiles["test.h.pch"] = "";
+
+  TU.ExtraArgs = {"-include-pch", "test.h.pch"};
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+  ->getPreprocessorOpts()
+  .ImplicitPCHInclude,
+  IsEmpty());
+
+  // Transparent include translation
+  TU.ExtraArgs = {"-include", "test.h"};
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+  ->getPreprocessorOpts()
+  .ImplicitPCHInclude,
+  IsEmpty());
+
+  // CL mode parsing.
+  TU.AdditionalFiles["test.pch"] = "";
+  TU.ExtraArgs = {"--driver-mode=cl"};
+  TU.ExtraArgs.push_back("/Yutest.h");
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+  ->getPreprocessorOpts()
+  .ImplicitPCHInclude,
+  IsEmpty());
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+  ->getPreprocessorOpts()
+  .PCHThroughHeader,
+  IsEmpty());
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -34,6 +34,7 @@
   CodeCompletionStringsTests.cpp
   CollectMacrosTests.cpp
   CompileCommandsTests.cpp
+  CompilerTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp
   DraftStoreTests.cpp
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -41,8 +41,7 @@
 }
 
 std::unique_ptr
-buildCompilerInvocation(const ParseInputs &Inputs,
-clang::DiagnosticConsumer &D,
+buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
 std::vector *CC1Args) {
   std::vector ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
@@ -74,6 +73,15 @@
   CI->getDependencyOutputOpts().HeaderIncludeOutputFile.clear();
   CI->getDependencyOutputOpts().DOTOutputFile.clear();
   CI->getDependencyOutputOpts().ModuleDependencyOutputDir.clear();
+
+  // Disable any pch generation/usage operations. Since serialized preamble
+  // format is unstable, using an incompatible one might result in unexpected
+  // behaviours, including crashes.
+  CI->getPreprocessorOpts().ImplicitPCHInclude.clear();
+  CI->getPreprocessorOpts().PrecompiledPreambleBytes = {0, false};
+  CI->getPreprocessorOpts().PCHThroughHeader.clear();
+  CI->getPreprocessorOpts().PCHWithHdrStop = false;
+  CI->getPreprocessorOpts().PCHWithHdrStopCreate = false;
   return CI;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79354: [clang-format] [PR34574] Handle [[nodiscard]] attribute in class declaration

2020-05-09 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31fd12aa0956: [clang-format] [PR34574] Handle [[nodiscard]] 
attribute in class declaration (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79354

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7647,6 +7647,10 @@
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aa\n"
"[[unused]] aaa(int i);");
+  verifyFormat("[[nodiscard]] bool f() { return false; }");
+  verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2399,9 +2399,10 @@
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, TT_AttributeSquare) ||
  ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
@@ -2421,8 +2422,16 @@
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros or attributes in between 'class' and the class name.
-if (!IsNonMacroIdentifier && FormatTok->Tok.is(tok::l_paren))
-  parseParens();
+if (!IsNonMacroIdentifier) {
+  if (FormatTok->Tok.is(tok::l_paren)) {
+parseParens();
+  } else if (FormatTok->is(TT_AttributeSquare)) {
+parseSquare();
+// Consume the closing TT_AttributeSquare.
+if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+  nextToken();
+  }
+}
   }
 
   // Note that parsing away template declarations here leads to incorrectly


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7647,6 +7647,10 @@
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aa\n"
"[[unused]] aaa(int i);");
+  verifyFormat("[[nodiscard]] bool f() { return false; }");
+  verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2399,9 +2399,10 @@
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, TT_AttributeSquare) ||
  ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
@@ -2421,8 +2422,16 @@
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros or attributes in between 'class' and the class name.
-if (!IsNonMacroIdentifier && FormatTok->Tok.is(tok::l_paren))
-  parseParens();
+if (!IsNonMacroIdentifier) {
+  if (FormatTok->Tok.is(tok::l_paren)) {
+parseParens();
+  } else if (FormatTok->is(TT_AttributeSquare)) {
+parseSquare();
+// Consume the closing TT_AttributeSquare.
+if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+  nextToken();
+  }
+}

[clang] 31fd12a - [clang-format] [PR34574] Handle [[nodiscard]] attribute in class declaration

2020-05-09 Thread via cfe-commits


Author: mydeveloperday
Date: 2020-05-09T11:27:23+01:00
New Revision: 31fd12aa09563d8402b1aefceaa2311b680e138c

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

LOG: [clang-format] [PR34574] Handle [[nodiscard]] attribute in class 
declaration

Summary:
https://bugs.llvm.org/show_bug.cgi?id=34574
https://bugs.llvm.org/show_bug.cgi?id=38401

```
template 
class [[nodiscard]] result
{
  public:
result(T&&)
{
}
};
```

formats incorrectly to

```
template 
class [[nodiscard]] result{public : result(T &&){}};
```

Reviewed By: krasimir

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 96e0bd2276fa..48a4d1d5b3ed 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2399,9 +2399,10 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, TT_AttributeSquare) ||
  ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
@@ -2421,8 +2422,16 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros or attributes in between 'class' and the class name.
-if (!IsNonMacroIdentifier && FormatTok->Tok.is(tok::l_paren))
-  parseParens();
+if (!IsNonMacroIdentifier) {
+  if (FormatTok->Tok.is(tok::l_paren)) {
+parseParens();
+  } else if (FormatTok->is(TT_AttributeSquare)) {
+parseSquare();
+// Consume the closing TT_AttributeSquare.
+if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+  nextToken();
+  }
+}
   }
 
   // Note that parsing away template declarations here leads to incorrectly

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 9a654d66250c..227e12dfaf73 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -7647,6 +7647,10 @@ TEST_F(FormatTest, UnderstandsSquareAttributes) {
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aa\n"
"[[unused]] aaa(int i);");
+  verifyFormat("[[nodiscard]] bool f() { return false; }");
+  verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"



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

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

2020-05-09 Thread Roland McGrath via Phabricator via cfe-commits

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mcgrathr added a parent revision: D79665: [Clang] Pass --pack-dyn-relocs=relr 
to lld for Fuchsia.

Currently all Fuchsia ABIs use a 4k page size, departing from
the recommended page sizes in the respective psABI documents.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79667

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

2020-05-09 Thread Roland McGrath via Phabricator via cfe-commits

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The compact format is fully supported on Fuchsia and is the
preferred default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79665

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79628: [Clang][Driver] Add Bounds and Thread to SupportsCoverage list

2020-05-09 Thread Vitaly Buka via Phabricator via cfe-commits

vitalybuka added a comment.

can you please add some basic IR test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79628



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

48 matches

Mail list logo