r357452 - SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Apr  2 01:01:38 2019
New Revision: 357452

URL: http://llvm.org/viewvc/llvm-project?rev=357452&view=rev
Log:
SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without 
used results (PR41259)

The code was previously checking that candidates for sinking had exactly
one use or were a store instruction (which can't have uses). This meant
we could sink call instructions only if they had a use.

That limitation seemed a bit arbitrary, so this patch changes it to
"instruction has zero or one use" which seems more natural and removes
the need to special-case stores.

Differential revision: https://reviews.llvm.org/D59936

Modified:
cfe/trunk/test/CodeGenCXX/nrvo.cpp
cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
cfe/trunk/test/CodeGenObjC/exceptions.m

Modified: cfe/trunk/test/CodeGenCXX/nrvo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/nrvo.cpp?rev=357452&r1=357451&r2=357452&view=diff
==
--- cfe/trunk/test/CodeGenCXX/nrvo.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/nrvo.cpp Tue Apr  2 01:01:38 2019
@@ -60,7 +60,6 @@ X test2(bool B) {
   // CHECK-NEXT: call void @llvm.lifetime.start
   // CHECK-NEXT: call {{.*}} @_ZN1XC1Ev
   // CHECK: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   // CHECK: call {{.*}} @_ZN1XD1Ev
   // CHECK-NEXT: call void @llvm.lifetime.end
   // CHECK: call {{.*}} @_ZN1XD1Ev

Modified: cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp?rev=357452&r1=357451&r2=357452&view=diff
==
--- cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp Tue Apr  2 01:01:38 
2019
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -o - -emit-llvm -O1 \
-// RUN: -fexceptions -fcxx-exceptions | FileCheck %s
+// RUN: -fexceptions -fcxx-exceptions -mllvm 
-simplifycfg-sink-common=false | FileCheck %s
 //
 // We should emit lifetime.ends for these temporaries in both the 'exception'
 // and 'normal' paths in functions.

Modified: cfe/trunk/test/CodeGenObjC/exceptions.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/exceptions.m?rev=357452&r1=357451&r2=357452&view=diff
==
--- cfe/trunk/test/CodeGenObjC/exceptions.m (original)
+++ cfe/trunk/test/CodeGenObjC/exceptions.m Tue Apr  2 01:01:38 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -O2 -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -mllvm 
-simplifycfg-sink-common=false -O2 -o - %s | FileCheck %s
 //
 //  [irgen] [eh] Exception code built with clang 
(x86_64) crashes
 


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

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

[Ilya Biryukov via Phabricator via cfe-commits]

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

Looks great, thanks!




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:110
+  // constructed with the builder class.
+  static constexpr char RootId[] = "___root___";
+};

NIT: `llvm::StringLiteral` is a vocabulary type with compile-time size that 
could be used here.
Although I don't think it has any actual benefits over char array, so leaving 
as is also looks good.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:113
+
+/// A fluent, builder class for \c RewriteRule.  See comments on \c 
RewriteRule,
+/// above.

NIT: comma seems redundant, this should probably be: `A fluent builder class`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

Re: [clang-tools-extra] r357429 - Fix clangd unittest _WIN32 ifdef

[Ilya Biryukov via cfe-commits]

Thanks for fixing this!

On Mon, Apr 1, 2019 at 11:14 PM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rnk
> Date: Mon Apr  1 14:16:17 2019
> New Revision: 357429
>
> URL: http://llvm.org/viewvc/llvm-project?rev=357429&view=rev
> Log:
> Fix clangd unittest _WIN32 ifdef
>
> WIN32 is not defined, _WIN32 is, use that instead.
>
> Modified:
> clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
>
> Modified: clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp?rev=357429&r1=357428&r2=357429&view=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp Mon
> Apr  1 14:16:17 2019
> @@ -17,8 +17,8 @@ namespace {
>
>  // No fmemopen on windows or on versions of MacOS X earlier than 10.13,
> so we
>  // can't easily run this test.
> -#if !(defined(WIN32) || (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) &&
>  \
> - __MAC_OS_X_VERSION_MIN_REQUIRED < 101300))
> +#if !(defined(_WIN32) || (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) &&
>   \
> +  __MAC_OS_X_VERSION_MIN_REQUIRED < 101300))
>
>  // Fixture takes care of managing the input/output buffers for the
> transport.
>  class JSONTransportTest : public ::testing::Test {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


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

[clang-tools-extra] r357454 - [clangd] Use capacity() instead of size() in RefSlab::bytes()

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Apr  2 01:24:37 2019
New Revision: 357454

URL: http://llvm.org/viewvc/llvm-project?rev=357454&view=rev
Log:
[clangd] Use capacity() instead of size() in RefSlab::bytes()

Patch by Nathan Ridge.

Reviewers: gribozavr

Reviewed By: gribozavr

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/Ref.h

Modified: clang-tools-extra/trunk/clangd/index/Ref.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Ref.h?rev=357454&r1=357453&r2=357454&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Ref.h (original)
+++ clang-tools-extra/trunk/clangd/index/Ref.h Tue Apr  2 01:24:37 2019
@@ -86,7 +86,7 @@ public:
 
   size_t bytes() const {
 return sizeof(*this) + Arena.getTotalMemory() +
-   sizeof(value_type) * Refs.size();
+   sizeof(value_type) * Refs.capacity();
   }
 
   /// RefSlab::Builder is a mutable container that can 'freeze' to RefSlab.


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

[PATCH] D60040: [clangd] Use capacity() instead of size() in RefSlab::bytes()

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357454: [clangd] Use capacity() instead of size() in 
RefSlab::bytes() (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60040?vs=192997&id=193241#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60040

Files:
  clang-tools-extra/trunk/clangd/index/Ref.h


Index: clang-tools-extra/trunk/clangd/index/Ref.h
===
--- clang-tools-extra/trunk/clangd/index/Ref.h
+++ clang-tools-extra/trunk/clangd/index/Ref.h
@@ -86,7 +86,7 @@
 
   size_t bytes() const {
 return sizeof(*this) + Arena.getTotalMemory() +
-   sizeof(value_type) * Refs.size();
+   sizeof(value_type) * Refs.capacity();
   }
 
   /// RefSlab::Builder is a mutable container that can 'freeze' to RefSlab.


Index: clang-tools-extra/trunk/clangd/index/Ref.h
===
--- clang-tools-extra/trunk/clangd/index/Ref.h
+++ clang-tools-extra/trunk/clangd/index/Ref.h
@@ -86,7 +86,7 @@
 
   size_t bytes() const {
 return sizeof(*this) + Arena.getTotalMemory() +
-   sizeof(value_type) * Refs.size();
+   sizeof(value_type) * Refs.capacity();
   }
 
   /// RefSlab::Builder is a mutable container that can 'freeze' to RefSlab.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina created this revision.
MarinaKalashina added reviewers: djasper, sylvestre.ledru, alexfh.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

This commit adds a chapter 'CLion integration' to ClangFormat.rst. 
The official announcement of clang-format support in CLion 2019.1: 
https://blog.jetbrains.com/clion/2019/03/clion-2019-1-embedded-dev-clangformat-memory-view/


Repository:
  rC Clang

https://reviews.llvm.org/D60115

Files:
  docs/ClangFormat.rst


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into CLion as an alternative code
+formatter. It is disabled by default and can be turned on in
+Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 


Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst
+++ docs/ClangFormat.rst
@@ -165,6 +165,19 @@
 shortcut in the BBEdit preferences, under Menus & Shortcuts.
 
 
+CLion Integration
+==
+
+:program:`clang-format` is integrated into CLion as an alternative code
+formatter. It is disabled by default and can be turned on in
+Settings/Preferences | Editor | Code Style.
+
+If :program:`clang-format` support is enabled, CLion detects config files when
+opening a project and suggests overriding the current IDE settings. Code style
+rules from the ``.clang-format`` files are then applied automatically to all
+editor actions, including auto-completion, code generation, and refactorings.
+
+
 Visual Studio Integration
 =
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60115: Adding 'CLion integration' to clang-format doc

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added inline comments.



Comment at: docs/ClangFormat.rst:171
+
+:program:`clang-format` is integrated into CLion as an alternative code
+formatter. It is disabled by default and can be turned on in

Maybe add a link to CLion here? I didn't know what it was :)



Repository:
  rC Clang

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

https://reviews.llvm.org/D60115



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

[PATCH] D58573: [analyzer] Move UninitializedObject out of alpha

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

One heuristic that we could use is to ignore inherited data members optionally. 
Does that sound any good?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58573



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

[PATCH] D31417: [OpenMP] Add support for omp simd pragmas without runtime

[Graham Hunter via Phabricator via cfe-commits]

huntergr abandoned this revision.
huntergr added a comment.

In D31417#1450587 , @xtian wrote:

> LGTM.  This provides a consistent behavior same as GCC and ICC w/ 
> -fopenmp-simd option. To answer, Kelvin's question. it is not directly tied 
> with "target".


Thanks for the review Xinmin, but as Alexey notes in the previous comment 
-fopenmp-simd support has already been committed -- I missed that comment 
earlier, but should have closed it then.

Apologies for the delay in implementing it Alexey, I just had too many other 
things to take care of at the time, and thanks for getting a better 
implementation in.


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

https://reviews.llvm.org/D31417



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Will address the rest of the comments later, answering a few questions that 
popped up first.




Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:72
+  /// macro or a name, arguments and parentheses of a function-like macro.
+  llvm::ArrayRef macroTokens(const TokenBuffer &B) const;
+  /// The range covering macroTokens().

sammccall wrote:
> ilya-biryukov wrote:
> > gribozavr wrote:
> > > `invocationTokens` or `macroInvocationTokens`
> > The plan is to eventually include the macro directives tokens, hence the 
> > name.
> > `invocationTokens` are somewhat inaccurate in that case, can't come up with 
> > a better name.
> I think your reply applies to TokenBuffer::macroTokens(), not 
> MacroInvocation::macroTokens().
> 
> +1 to invocationTokens here.
Yeah, sorry, I have mixed up these two functions with the same name. Will 
change to `invocationTokens`.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:1
+//===- TokenBuffer.h - store tokens of preprocessed files -*- C++ 
-*-=//
+//

sammccall wrote:
> are you sure TokenBuffer is the central concept in this file, rather than 
> just the thing with the most code? Token.h might end up being a better name 
> for users.
I don't mind changing this to `Token.h`, although I'd personally expect that a 
file with this name only contains a definition for the token class.
`Tokens.h` would be a better fit from my POV. WDYT?



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:92
+
+/// A list of tokens obtained by lexing and preprocessing a text buffer and a
+/// set of helpers to allow mapping the tokens after preprocessing to the

sammccall wrote:
> Warning, braindump follows - let's discuss further.
> We talked a bunch offline about the logical and concrete data model here.
> 
> As things stand:
>  - #includes are not expanded, but will refer to a file ID with its own 
> buffer: `map` is the whole structure
>  - no information is captured about other PP interactions (PP directives that 
> generate no tokens, skipped sections
>  - the spelled sequence of tokens is not directly available (but expanded + 
> macro invocations may be enough to reconstruct it)
> 
> If we keep this model, I think we should spell both of these out in the 
> comment.
> But there's another fairly natural model we could consider:
> 
>  - we have a single TokenBuffer with a flat list of all expanded tokens in 
> the TU, rather than for the file 
>  - one FileID corresponds to a contiguous range of *transitive* symbols, 
> these ranges nest
>  - the TokenBuffer also stores the original tokens as `map vector>`
>  - spelled -> expanded token mapping: spelled tokens for a file are 
> partitioned into ranges (types: literal, include, macro invocation, other pp 
> directive, pp skipped region). each range maps onto a range of expanded 
> tokens (empty for e.g. pp directive or skipped region)
>  - expanded -> spelled token is similar. (It's almost the inverse of the of 
> the other mapping, we drop the "include" mapping ranges)
>  - This can naturally handle comments, preprocessor directives, pp skipped 
> sections, etc - these are in the spelled stream but not the expanded stream.
> 
> e.g. for this code
> ```
> // foo.cpp
> int a;
> #include "foo.h"
> int b =  FOO(42);
> // foo.h
> #define FOO(X) X*X
> int c;
> ```
> 
> we'd have this buffer:
> ```
> expandedTokens = [int a ; int c ; int b = 42 * 42 ;]
> spelledTokens = {
>   "foo.cpp" => [int a; # include "foo.h" int b = FOO ( 42 ) ;],
>   "foo.h" => [# define FOO ( X ) X * X int c ;]
> }
> 
> expandedToSpelling = {
>   int => int (foo.cpp), type = literal
>   a => a
>   ; => ;
> 
>   [] => [# define FOO ( X ) X * X](foo.h), type=preprocessor directive
>   int => int (foo.h)
>   c => c
>   ; => ;
> 
>   int => int (foo.cpp)
>   b => b
>   = => =
>   [42 * 42] => [FOO ( 42 ) ](foo.cpp), type=macro invocation
>   ; => ; (foo.cpp)
> }
> 
> spellingToExpanded = {
>   // foo.cpp
>   int => int, type=literal
>   a => a
>   ; => ;
>   [# include "foo.h"] => [int c ;], type=include
>   int => int
>   b => b
>   = => =
>   [FOO ( X )] => [42 * 42], type=macro invocation
>   ; => ;
> 
>   // foo.h
>   [# define FOO ( X ) X] => [], type=preprocessor directive
>   int => int
>   c => c
>   ; => ;
> }
> ```
> 
> Various implementation strategies possible here, one obvious one is to use a 
> flat sorted list, and include a sequence of literal tokens as a single entry. 
> 
> ```
> struct ExpandedSpellingMapping {
>   unsigned ExpandedStart, ExpandedEnd;
>   FileID SpellingFile; // redundant for illustration: can actually derive 
> from SpellingTokens[SpellingStart].location()
>   unsigned SpellingStart, SpellingEnd;
>   enum { LiteralSequence, MacroInvocation, Preprocessor, PPSkipped, Inclusion 
> } Kind;
> }
> vector ExpandedToSpelling; // bsearchable
> vector> Inclusions; // for spelling -> 

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi,

I noticed that with this commit I get a whole bunch (~40) of warnings like the 
below when compiling with gcc 7.4:

  [10/16] Building CXX object 
tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/clangTidyUtils.dir/HeaderGuard.cpp.o
  In file included from 
../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:12:0,
   from 
../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:
  ../tools/clang/tools/extra/clang-tidy/utils/../ClangTidy.h:161:16: warning: 
'virtual void clang::tidy::ClangTidyCheck::registerPPCallbacks(const 
clang::SourceManager&, clang::Preprocessor*, clang::Preprocessor*)' was hidden 
[-Woverloaded-virtual]
 virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
  ^~~
  In file included from 
../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:0:
  ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:35:8: warning:   by 
'virtual void 
clang::tidy::utils::HeaderGuardCheck::registerPPCallbacks(clang::CompilerInstance&)'
 [-Woverloaded-virtual]
 void registerPPCallbacks(CompilerInstance &Compiler) override;
  ^~~

One of the registerPPCallbacks methods has the following comment

  /// DEPRECATED: Use the other overload.
  virtual void registerPPCallbacks(CompilerInstance &Compiler) {}

so I suppose it will be removed at some point?

I'm not super familiar with the warning, but there is at least one discussion 
about it here:
https://stackoverflow.com/questions/9995421/gcc-woverloaded-virtual-warnings


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59528



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTImporter.cpp:2154
+return NameOrErr.takeError();
 }
   }

a_sidorin wrote:
> Should we write `else Name = *NameOrError`?
Atm, we implement the simplest strategy for name conflict handling: we just 
return with the error.
However, on a long term we should use the returned name indeed. I mean when we 
really do implement a renaming strategy via `HandleNameConflict`. 

Also, for that we'd have to double check that the `Name` is indeed used when we 
create the AST node. So I'd rather leave this `else` branch up to that point. 
Hopefully, by that time we'll have unittests which would exercise this `else` 
branch, now we just don't have any.



Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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

Re: [PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Alexander Kornienko via cfe-commits]

On Tue, Apr 2, 2019 at 10:53 AM Mikael Holmén via Phabricator <
revi...@reviews.llvm.org> wrote:

> uabelho added a comment.
>
> Hi,
>
> I noticed that with this commit I get a whole bunch (~40) of warnings like
> the below when compiling with gcc 7.4:
>
>   [10/16] Building CXX object
> tools/clang/tools/extra/clang-tidy/utils/CMakeFiles/clangTidyUtils.dir/HeaderGuard.cpp.o
>   In file included from
> ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:12:0,
>from
> ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:
>   ../tools/clang/tools/extra/clang-tidy/utils/../ClangTidy.h:161:16:
> warning: 'virtual void
> clang::tidy::ClangTidyCheck::registerPPCallbacks(const
> clang::SourceManager&, clang::Preprocessor*, clang::Preprocessor*)' was
> hidden [-Woverloaded-virtual]
>  virtual void registerPPCallbacks(const SourceManager &SM,
> Preprocessor *PP,
>   ^~~
>   In file included from
> ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.cpp:9:0:
>   ../tools/clang/tools/extra/clang-tidy/utils/HeaderGuard.h:35:8:
> warning:   by 'virtual void
> clang::tidy::utils::HeaderGuardCheck::registerPPCallbacks(clang::CompilerInstance&)'
> [-Woverloaded-virtual]
>  void registerPPCallbacks(CompilerInstance &Compiler) override;
>   ^~~
>
> One of the registerPPCallbacks methods has the following comment
>
>   /// DEPRECATED: Use the other overload.
>   virtual void registerPPCallbacks(CompilerInstance &Compiler) {}
>
> so I suppose it will be removed at some point?
>

Indeed, the plan is to remove the old overload.


>
> I'm not super familiar with the warning, but there is at least one
> discussion about it here:
>
> https://stackoverflow.com/questions/9995421/gcc-woverloaded-virtual-warnings
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59528/new/
>
> https://reviews.llvm.org/D59528
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60116: [clang-format] Regroup #includes into blocks for Google style

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: sammccall, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Regrouping #includes in blocks separated by blank lines when sorting C++ 
#include
headers was implemented recently, and it has been preferred in Google's C++ 
style guide:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes


Repository:
  rC Clang

https://reviews.llvm.org/D60116

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/SortIncludesTest.cpp


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -272,6 +272,7 @@
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
 sort("#include \n"
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;


Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -272,6 +272,7 @@
   FmtStyle = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include \n"
 "#include \n"
+"\n"
 "#include \"a.h\"\n"
 "#include \"c.h\"\n",
 sort("#include \n"
Index: unittests/Format/CleanupTest.cpp
===
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -420,8 +420,10 @@
 TEST_F(CleanUpReplacementsTest, InsertMultipleNewHeadersAndSortGoogle) {
   std::string Code = "\nint x;";
   std::string Expected = "\n#include \"fix.h\"\n"
+ "\n"
  "#include \n"
  "#include \n"
+ "\n"
  "#include \"a.h\"\n"
  "#include \"b.h\"\n"
  "#include \"c.h\"\n"
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -784,6 +784,7 @@
   GoogleStyle.IncludeStyle.IncludeCategories = {
   {"^", 2}, {"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:75
   Context.diag(CheckName, PD->getLocation().asLocation(),
-   PD->getShortDescription())
+   PD->getShortDescription(), /*FixDescription*/ "")
   << PD->path.back()->getRanges();

nit: Use the format that the bugprone-argument-comment check understands: 
`/*FixDescription=*/`

Same below.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:109
+  /// not supported.
+  virtual llvm::StringRef fixDescription() { return ""; }
+

Checks can provide different fixes with different semantics depending on the 
context. It seems incorrect to assume that there can be one reasonable 
description for all of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith : up :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59413



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

r357461 - Fix compiler warning, remove extra ";" [NFC]

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Tue Apr  2 03:01:09 2019
New Revision: 357461

URL: http://llvm.org/viewvc/llvm-project?rev=357461&view=rev
Log:
Fix compiler warning, remove extra ";" [NFC]

At least gcc 7.4 complained with
../tools/clang/lib/StaticAnalyzer/Checkers/Taint.cpp:26:53: warning: extra ';' 
[-Wpedantic]
TaintTagType);
 ^

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp?rev=357461&r1=357460&r2=357461&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp Tue Apr  2 03:01:09 2019
@@ -23,7 +23,7 @@ REGISTER_MAP_WITH_PROGRAMSTATE(TaintMap,
 
 // Partially tainted symbols.
 REGISTER_MAP_FACTORY_WITH_PROGRAMSTATE(TaintedSubRegions, const SubRegion *,
-   TaintTagType);
+   TaintTagType)
 REGISTER_MAP_WITH_PROGRAMSTATE(DerivedSymTaint, SymbolRef, TaintedSubRegions)
 
 void taint::printTaint(ProgramStateRef State, raw_ostream &Out, const char *NL,


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

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

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added a comment.

This intrinsic got added to gcc a while ago and should become available in the 
upcoming gcc 9 release.
In gcc however, the prototype of the intrinsic is slightly different (see 
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html):
type __builtin_speculation_safe_value (type val, type failval)
It provides a second optional argument "failval". From the gcc documentation: 
"The function may use target-dependent speculation tracking state to cause 
failval to be returned when it is known that speculative execution has 
incorrectly predicted a conditional branch operation."
So, when implementing the intrinsic using a speculation barrier such as lfence, 
that failval argument doesn't have any effect. However, when lowering the 
intrinsic using speculation tracking similar to how that's used in SLH, this 
failval parameter is used to return a non-zero value on miss-speculation, in 
case the developer prefers that over the default zero value.

I think we should make the intrinsic compatible with the one introduced in gcc.




Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:1
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s 
--check-prefix=X64
+

I guess the -mtriple command line option may not be needed since the IR file 
contain "target triple" and "target datalayout" information?



Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:3-4
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

I guess this is not strictly necessary for this test, so should be removed?



Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:8-62
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i32, align 4
+  %b_safe = alloca i32, align 4
+  %c = alloca i32, align 4

Thanks for those updates, Zola. It makes it easier to compare this patch with 
the code I wrote earlier.
Doing that comparison, I see that I had a few changes too in target-independent 
SelectionDAG under lib/Codegen/SelectionDAG.
IIRC, you might find that you'll need that code if you also add tests here to 
test the correct thing happens when applying the intrinsic on other types than 
i32 or i64.
You probably also would want a test on a pointer data type here, I guess.



Comment at: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll:64-71
+attributes #0 = { noinline nounwind optnone uwtable 
"correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" 
"less-precise-fpmad"="false" "min-legal-vector-width"="0" 
"no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" 
"no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" 
"no-signed-zeros-fp-math"="false" "no-trapping-math"="false" 
"stack-protector-buffer-size"="8" "target-cpu"="x86-64" 
"target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" 
"use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}

I guess this is not strictly necessary for this test, so should be removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59827



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

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Are you planning to do this recursively?
The minimizer does not help much for the following example, while Sema.h 
contains 10,000 lines of useless code.

  #include "clang/Sema/Sema.h"
  
  int foo() {}


Repository:
  rC Clang

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

https://reviews.llvm.org/D55463



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/CFG.cpp:2208
+}
+
 CFGBlock *CFGBuilder::VisitStmt(Stmt *S, AddStmtChoice asc) {

NoQ wrote:
> Tyker wrote:
> > NoQ wrote:
> > > Tyker wrote:
> > > > riccibruno wrote:
> > > > > I don't understand why this is needed. Can you explain it ? Also I 
> > > > > think that someone familiar with this code should comment on this 
> > > > > (maybe @NoQ ?)
> > > > the detail of why are complicated and i don't have them all in head but 
> > > > without this edit in cases like 
> > > > 
> > > > ```
> > > > switch (...) {
> > > > [[likely]] case 1:
> > > > ...
> > > > [[fallthrough]];
> > > > default:
> > > > ...
> > > > }
> > > > ```
> > > > the fallthrough attribute emitted a diagnostic because is wasn't 
> > > > handling attributed case statement. the edit i performed is probably 
> > > > not the optimal way to solve the issue as it only solves the issue for 
> > > > likelihood attribute. but i don't know any other attribute that can be 
> > > > applied on a case statement but if they were others they would probably 
> > > > have the same issue. but the code is quite hard to follow and i didn't 
> > > > wanted to break anything. so this is what i came up with.
> > > > i am going to look into it to find a better solution.
> > > The [[likely]] attribute should not affect the overall topology of the 
> > > CFG. It might be a nice piece of metadata to add to a CFG edge or to a 
> > > CFG terminator, but for most consumers of the CFG (various static 
> > > analyses such as analysis-based warnings or the Static Analyzer) the 
> > > attribute should have little to no effect - the tool would still need to 
> > > explore both branches. I don't know how exactly the fallthrough warning 
> > > operates, but i find it likely (no pun intended) that the fallthrough 
> > > warning itself should be updated, not the CFG.
> > > 
> > > It is probable that for compiler warnings it'll only cause false 
> > > negatives, which is not as bad as false positives, but i wouldn't rely on 
> > > that. Additionally, false negatives in such rare scenarios will be very 
> > > hard to notice later. So i'm highly in favor of aiming for the correct 
> > > solution in this patch.
> > > 
> > > 
> > i think we all agree that the CFG structure shouldn't be affected by the 
> > presence or absence of the likely attribute. but in the current state(no 
> > changes to the CFG) it does for example. 
> > 
> > the CFG were obtained without the edit in CFG.cpp but with the added likely 
> > attribute
> > using: clang -cc1 -analyze test.cpp -analyzer-checker=debug.DumpCFG
> > 
> > input:
> > 
> > ```
> > int f(int i) {
> > switch (i) {
> > [[likely]] case 1:
> > return 1;
> > }
> > return i;
> > }
> > 
> > ```
> > outputs:
> > 
> > ```
> >  [B5 (ENTRY)]
> >Succs (1): B2
> > 
> >  [B1]
> >1: i
> >2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
> >3: return [B1.2];
> >Preds (2): B3 B2
> >Succs (1): B0
> > 
> >  [B2]
> >1: i
> >2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
> >T: switch [B2.2]
> >Preds (1): B5
> >Succs (2): B4 B1
> > 
> >  [B3]
> >1:  [[likely]]case 1:
> > [B4.2]   Succs (1): B1
> > 
> >  [B4]
> >   case 1:
> >1: 1
> >2: return [B4.1];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >Preds (2): B1 B4
> > 
> > ```
> > and
> > input:
> > 
> > ```
> > int f(int i) {
> > switch (i) {
> >  case 1:
> > return 1;
> > }
> > return i;
> > }
> > 
> > ```
> > outputs:
> > 
> > ```
> >  [B4 (ENTRY)]
> >Succs (1): B2
> > 
> >  [B1]
> >1: i
> >2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
> >3: return [B1.2];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B2]
> >1: i
> >2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
> >T: switch [B2.2]
> >Preds (1): B4
> >Succs (2): B3 B1
> > 
> >  [B3]
> >   case 1:
> >1: 1
> >2: return [B3.1];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >Preds (2): B1 B3
> > ```
> > i think think this is the underlying issue. the false diagnostic from 
> > fallthrough previously mentioned is a consequence of this
> Hmm, i see. I got it all wrong. Please forgive me!
> 
> You're just trying to make `CFGBuilder` support `AttributedStmt` correctly in 
> general. And the logic you're writing says "support it as any other generic 
> Stmt that doesn't have any control flow in it, unless it's a 
> `[[likely]]`-attributed `AttributedStmt`, in which case jump straight to 
> children".
> 
> Could you instead do this by implementing `CFGBuilder::VisitAttributedStmt` 
> with that logic?
> 
> I'm also not sure this logic is actually specific to `[[likely]]`. Maybe we 
> should unwrap all `AttributedStmt`s similarly?
we shouldn't handle all Attribu

r357466 - [PowerPC] Fix issue with inline asm - soft float mode

[Strahinja Petrovic via cfe-commits]

Author: spetrovic
Date: Tue Apr  2 04:00:09 2019
New Revision: 357466

URL: http://llvm.org/viewvc/llvm-project?rev=357466&view=rev
Log:
[PowerPC] Fix issue with inline asm - soft float mode

This patch prevents floating point register
constraints in soft float mode.

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

Added:
cfe/trunk/test/Driver/ppc-inlineasm-sf.c
Modified:
cfe/trunk/lib/Basic/Targets/PPC.cpp
cfe/trunk/lib/Basic/Targets/PPC.h

Modified: cfe/trunk/lib/Basic/Targets/PPC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.cpp?rev=357466&r1=357465&r2=357466&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.cpp Tue Apr  2 04:00:09 2019
@@ -30,6 +30,7 @@ const Builtin::Info PPCTargetInfo::Built
 /// configured set of features.
 bool PPCTargetInfo::handleTargetFeatures(std::vector &Features,
  DiagnosticsEngine &Diags) {
+  FloatABI = HardFloat;
   for (const auto &Feature : Features) {
 if (Feature == "+altivec") {
   HasAltivec = true;
@@ -53,6 +54,8 @@ bool PPCTargetInfo::handleTargetFeatures
   HasFloat128 = true;
 } else if (Feature == "+power9-vector") {
   HasP9Vector = true;
+} else if (Feature == "-hard-float") {
+  FloatABI = SoftFloat;
 }
 // TODO: Finish this list and add an assert that we've handled them
 // all.

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=357466&r1=357465&r2=357466&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Tue Apr  2 04:00:09 2019
@@ -53,6 +53,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetI
   static const char *const GCCRegNames[];
   static const TargetInfo::GCCRegAlias GCCRegAliases[];
   std::string CPU;
+  enum PPCFloatABI { HardFloat, SoftFloat } FloatABI;
 
   // Target cpu features.
   bool HasAltivec = false;
@@ -183,8 +184,11 @@ public:
   return false;
 case 'O': // Zero
   break;
-case 'b': // Base register
 case 'f': // Floating point register
+  // Don't use floating point registers on soft float ABI.
+  if (FloatABI == SoftFloat)
+return false;
+case 'b': // Base register
   Info.setAllowsRegister();
   break;
 // FIXME: The following are added to allow parsing.
@@ -192,6 +196,10 @@ public:
 // Also, is more specific checking needed?  I.e. specific registers?
 case 'd': // Floating point register (containing 64-bit value)
 case 'v': // Altivec vector register
+  // Don't use floating point and altivec vector registers
+  // on soft float ABI
+  if (FloatABI == SoftFloat)
+return false;
   Info.setAllowsRegister();
   break;
 case 'w':

Added: cfe/trunk/test/Driver/ppc-inlineasm-sf.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ppc-inlineasm-sf.c?rev=357466&view=auto
==
--- cfe/trunk/test/Driver/ppc-inlineasm-sf.c (added)
+++ cfe/trunk/test/Driver/ppc-inlineasm-sf.c Tue Apr  2 04:00:09 2019
@@ -0,0 +1,16 @@
+// RUN: not %clang -target powerpc-unknown-linux -O2 -fPIC -m32 -msoft-float 
%s -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-ERRMSG %s
+int foo ()
+{
+  double x,y;
+  int a;
+  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  // CHECK-ERRMSG:  error: invalid output constraint '=f' in asm
+  // CHECK-ERRMSG-NEXT:  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  // CHECK-ERRMSG: error: invalid output constraint '=d' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  __asm__ ("vec_dss %0" : "=v"(a));
+  // CHECK-ERRMSG: error: invalid output constraint '=v' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("vec_dss %0" : "=v"(a));
+}
+


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

[PATCH] D59310: [PowerPC] Fix issue with inline asm - soft float mode

[Strahinja Petrovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357466: [PowerPC] Fix issue with inline asm - soft float 
mode (authored by spetrovic, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59310?vs=190446&id=193252#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59310

Files:
  lib/Basic/Targets/PPC.cpp
  lib/Basic/Targets/PPC.h
  test/Driver/ppc-inlineasm-sf.c


Index: test/Driver/ppc-inlineasm-sf.c
===
--- test/Driver/ppc-inlineasm-sf.c
+++ test/Driver/ppc-inlineasm-sf.c
@@ -0,0 +1,16 @@
+// RUN: not %clang -target powerpc-unknown-linux -O2 -fPIC -m32 -msoft-float 
%s -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-ERRMSG %s
+int foo ()
+{
+  double x,y;
+  int a;
+  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  // CHECK-ERRMSG:  error: invalid output constraint '=f' in asm
+  // CHECK-ERRMSG-NEXT:  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  // CHECK-ERRMSG: error: invalid output constraint '=d' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  __asm__ ("vec_dss %0" : "=v"(a));
+  // CHECK-ERRMSG: error: invalid output constraint '=v' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("vec_dss %0" : "=v"(a));
+}
+
Index: lib/Basic/Targets/PPC.cpp
===
--- lib/Basic/Targets/PPC.cpp
+++ lib/Basic/Targets/PPC.cpp
@@ -30,6 +30,7 @@
 /// configured set of features.
 bool PPCTargetInfo::handleTargetFeatures(std::vector &Features,
  DiagnosticsEngine &Diags) {
+  FloatABI = HardFloat;
   for (const auto &Feature : Features) {
 if (Feature == "+altivec") {
   HasAltivec = true;
@@ -53,6 +54,8 @@
   HasFloat128 = true;
 } else if (Feature == "+power9-vector") {
   HasP9Vector = true;
+} else if (Feature == "-hard-float") {
+  FloatABI = SoftFloat;
 }
 // TODO: Finish this list and add an assert that we've handled them
 // all.
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -53,6 +53,7 @@
   static const char *const GCCRegNames[];
   static const TargetInfo::GCCRegAlias GCCRegAliases[];
   std::string CPU;
+  enum PPCFloatABI { HardFloat, SoftFloat } FloatABI;
 
   // Target cpu features.
   bool HasAltivec = false;
@@ -183,8 +184,11 @@
   return false;
 case 'O': // Zero
   break;
-case 'b': // Base register
 case 'f': // Floating point register
+  // Don't use floating point registers on soft float ABI.
+  if (FloatABI == SoftFloat)
+return false;
+case 'b': // Base register
   Info.setAllowsRegister();
   break;
 // FIXME: The following are added to allow parsing.
@@ -192,6 +196,10 @@
 // Also, is more specific checking needed?  I.e. specific registers?
 case 'd': // Floating point register (containing 64-bit value)
 case 'v': // Altivec vector register
+  // Don't use floating point and altivec vector registers
+  // on soft float ABI
+  if (FloatABI == SoftFloat)
+return false;
   Info.setAllowsRegister();
   break;
 case 'w':


Index: test/Driver/ppc-inlineasm-sf.c
===
--- test/Driver/ppc-inlineasm-sf.c
+++ test/Driver/ppc-inlineasm-sf.c
@@ -0,0 +1,16 @@
+// RUN: not %clang -target powerpc-unknown-linux -O2 -fPIC -m32 -msoft-float %s -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-ERRMSG %s
+int foo ()
+{
+  double x,y;
+  int a;
+  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  // CHECK-ERRMSG:  error: invalid output constraint '=f' in asm
+  // CHECK-ERRMSG-NEXT:  __asm__ ("fctiw %0,%1" : "=f"(x) : "f"(y));
+  __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  // CHECK-ERRMSG: error: invalid output constraint '=d' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("fctiw %0,%1" : "=d"(x) : "d"(y));
+  __asm__ ("vec_dss %0" : "=v"(a));
+  // CHECK-ERRMSG: error: invalid output constraint '=v' in asm
+  // CHECK-ERRMSG-NEXT: __asm__ ("vec_dss %0" : "=v"(a));
+}
+
Index: lib/Basic/Targets/PPC.cpp
===
--- lib/Basic/Targets/PPC.cpp
+++ lib/Basic/Targets/PPC.cpp
@@ -30,6 +30,7 @@
 /// configured set of features.
 bool PPCTargetInfo::handleTargetFeatures(std::vector &Features,
  DiagnosticsEngine &Diags) {
+  FloatABI = HardFloat;
   for (const auto &Feature : Features) {
 if (Feature == "+altivec") {
   HasAltivec = true;
@@ -53,6 +54,8 @@
   HasFloat128 = true;
 } else if (Feature == "+power9-vector") {
   HasP9Vector = true;
+} else if (Feature == "-hard-

r357467 - Fix Wimplicit-fallthrough warning introduced in rL357466. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Apr  2 04:25:38 2019
New Revision: 357467

URL: http://llvm.org/viewvc/llvm-project?rev=357467&view=rev
Log:
Fix Wimplicit-fallthrough warning introduced in rL357466. NFCI.

Modified:
cfe/trunk/lib/Basic/Targets/PPC.h

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=357467&r1=357466&r2=357467&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Tue Apr  2 04:25:38 2019
@@ -188,6 +188,7 @@ public:
   // Don't use floating point registers on soft float ABI.
   if (FloatABI == SoftFloat)
 return false;
+  LLVM_FALLTHROUGH;
 case 'b': // Base register
   Info.setAllowsRegister();
   break;


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

[clang-tools-extra] r357468 - [clang-tidy] make getLangOpts return a const ref

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Apr  2 04:31:56 2019
New Revision: 357468

URL: http://llvm.org/viewvc/llvm-project?rev=357468&view=rev
Log:
[clang-tidy] make getLangOpts return a const ref

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h?rev=357468&r1=357467&r2=357468&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyCheck.h Tue Apr  2 04:31:56 2019
@@ -188,7 +188,7 @@ protected:
   /// \brief Returns the main file name of the current translation unit.
   StringRef getCurrentMainFile() const { return Context->getCurrentFile(); }
   /// \brief Returns the language options from the context.
-  LangOptions getLangOpts() const { return Context->getLangOpts(); }
+  const LangOptions &getLangOpts() const { return Context->getLangOpts(); }
 };
 
 } // namespace tidy


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

[PATCH] D60112: [analyzer] Treat write into a top-level parameter variable with destructor as escape.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Woah, the code looks amazing, cheers on the refactoring! I'll be honest, I'm 
struggling a bit with the sentence "we're now in the top frame". In order, I 
don't understand what does

- we
- now
- in the top frame

mean. "Top-level argument" is another one -- Do we have **precise** definitions 
for there terms?




Comment at: clang/test/Analysis/malloc.cpp:151
+  char *getName() {
+if (!name) {
+  name = static_cast(malloc(10));

Is this relevant? `name` will never be null.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60112



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

[PATCH] D60110: [analyzer] When failing to evaluate a __builtin_constant_p, presume it's false.

[Kristóf Umann via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60110



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

[PATCH] D60120: check-clang-tools: Actually build and run XPC test

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added reviewers: jkorous, arphaman.
Herald added subscribers: kadircet, dexonsmith, ioeric, ilya-biryukov, mgorny.

The CMake variable controlling if XPC code is built is called 
`CLANGD_BUILD_XPC` but three places unintentionally checked the non-existent 
variable `CLANGD_BUILD_XPC_SUPPORT` instead, which (due to being nonexistent, 
and due to cmake) always silently evaluated to false.

Luckily the test still seems to pass, despite never running after being added 
almost 3 months ago in r351280.


https://reviews.llvm.org/D60120

Files:
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/lit.site.cfg.py.in


Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -11,7 +11,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
-config.clangd_xpc_support = @CLANGD_BUILD_XPC_SUPPORT@
+config.clangd_xpc_support = @CLANGD_BUILD_XPC@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,7 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_ENABLE_STATIC_ANALYZER
-  CLANGD_BUILD_XPC_SUPPORT)
+  CLANGD_BUILD_XPC)
 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
@@ -70,7 +70,7 @@
   clang
 )
 
-if(CLANGD_BUILD_XPC_SUPPORT)
+if(CLANGD_BUILD_XPC)
   list(APPEND CLANG_TOOLS_TEST_DEPS clangd-xpc-test-client)
 endif()
 


Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -11,7 +11,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
-config.clangd_xpc_support = @CLANGD_BUILD_XPC_SUPPORT@
+config.clangd_xpc_support = @CLANGD_BUILD_XPC@
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -17,7 +17,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_ENABLE_STATIC_ANALYZER
-  CLANGD_BUILD_XPC_SUPPORT)
+  CLANGD_BUILD_XPC)
 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
@@ -70,7 +70,7 @@
   clang
 )
 
-if(CLANGD_BUILD_XPC_SUPPORT)
+if(CLANGD_BUILD_XPC)
   list(APPEND CLANG_TOOLS_TEST_DEPS clangd-xpc-test-client)
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 193258.
zahiraam marked 15 inline comments as done.

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

https://reviews.llvm.org/D43576

Files:
  include/clang/AST/Decl.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/Attr.td
  include/clang/Basic/DeclNodes.td
  include/clang/Sema/ParsedAttr.h
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/AST/DeclBase.cpp
  lib/AST/DeclCXX.cpp
  lib/CodeGen/CGDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTCommon.cpp
  test/Parser/MicrosoftExtensions.cpp
  test/Sema/ms-uuid-1.cpp
  test/Sema/ms-uuid-2.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -321,12 +321,15 @@
 OS << "\" << get" << getUpperName() << "().getAsString() << \"";
   else if (type == "ParamIdx")
 OS << "\" << get" << getUpperName() << "().getSourceIndex() << \"";
-  else
+  else if (type == "DeclSpecUuidDecl *") {
+OS << "\" << get" << getUpperName() << "() << \"";
+  } else
 OS << "\" << get" << getUpperName() << "() << \"";
 }
 
 void writeDump(raw_ostream &OS) const override {
-  if (type == "FunctionDecl *" || type == "NamedDecl *") {
+  if (type == "FunctionDecl *" || type == "NamedDecl *" ||
+  type == "DeclSpecUuidDecl *") {
 OS << "OS << \" \";\n";
 OS << "dumpBareDeclRef(SA->get" << getUpperName() << "());\n"; 
   } else if (type == "IdentifierInfo *") {
@@ -1296,6 +1299,8 @@
 Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VersionArgument")
 Ptr = llvm::make_unique(Arg, Attr);
+  else if (ArgName == "DeclSpecUuidDeclArgument")
+Ptr = llvm::make_unique(Arg, Attr, "DeclSpecUuidDecl *");
 
   if (!Ptr) {
 // Search in reverse order so that the most-derived type is handled first.
Index: test/Sema/ms-uuid-2.cpp
===
--- /dev/null
+++ test/Sema/ms-uuid-2.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions -fms-compatibility -std=c++14  %s
+
+
+typedef struct _GUID
+{
+unsigned long  Data1;
+unsigned short Data2;
+unsigned short Data3;
+unsigned char  Data4[8];
+} GUID;
+
+// expected-error@+5 {{C++ requires a type specifier for all declarations}}
+// expected-error@+4 {{invalid digit 'a' in decimal constant}}
+// expected-error@+3 {{use of undeclared identifier 'def0'}}
+// expected-error@+2 {{invalid digit 'a' in decimal constant}}
+// expected-error@+1 {{expected ';' after top level declarator}}
+uuid(12345678-9abc-def0-1234-56789abcdef0) struct S2;
+
Index: test/Sema/ms-uuid-1.cpp
===
--- /dev/null
+++ test/Sema/ms-uuid-1.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions -fms-compatibility -std=c++14 %s
+// expected-no-diagnostics
+typedef struct _GUID {
+  int i;
+} IID;
+template 
+class A {};
+
+struct
+__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
+S1 {};
+
+struct
+__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
+S2 {};
+
+struct __declspec(dllexport)
+C1 : public A<&__uuidof(S1)> {};
+
+struct __declspec(dllexport)
+C2 : public A<&__uuidof(S2)> {};
+int printf(const char *, ...);
+int main() {
+
+  if (&__uuidof(S1) == &__uuidof(S2))
+printf("OK\n");
+  else
+printf("ERROR\n");
+
+  return 0;
+}
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -76,7 +76,7 @@
struct_with_uuid var_with_uuid[1];
struct_without_uuid var_without_uuid[1];
 
-   __uuidof(struct_with_uuid);
+   __uuidof(struct_with_uuid); // expected-error {{non-type template argument of type 'const _GUID' is not a constant expression}}
__uuidof(struct_with_uuid2);
__uuidof(struct_with_uuid3);
__uuidof(struct_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
@@ -137,7 +137,7 @@
 
 COM_CLASS_TEMPLATE_REF good_template_arg;
 
-COM_CLASS_TEMPLATE bad_template_arg; // expected-error {{non-type template argument of type 'const _GUID' is not a constant expression}}
+COM_CLASS_TEMPLATE bad_template_arg;
 
 namespace PR16911 {
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
Index: lib/Serialization/ASTCommon.cpp
===
--- lib/Serialization/ASTCommon.cpp
+++ lib/Serialization/ASTCommon.cpp
@@ -348,6 +348,7 @@
   case Decl::ObjCProtocol:
   case Decl::ObjCInterface:
   case Decl:

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

There are still a few things I haven't addressed yet. Mostly because not sure 
there is another solution like getting rid of the map from StringRef to expr. 
The other issue is not adding new kind to ParsedAttr. There may be another way 
of doing it, but didn't look at it yet.
Meanwhile can you please let me know if you are happy with the current status 
of the implementation.




Comment at: lib/Parse/ParseDecl.cpp:594
+  bool Invalid = false;
+  StringRef UuidStr = PP.getSpelling(Tok, UuidBuffer, &Invalid);
+

rsmith wrote:
> How does this handle the case where multiple tokens must be concatenated to 
> form a uuid? Eg, `uuid(12345678-9abc-def0-1234-56789abcdef0)`
ksh-3.2$ cat m3.cpp
struct __declspec (uuid(12345678-9abc-def0-1234-56789abcdef0)) s;
ksh-3.2$ clang -c m3.cpp
m3.cpp:1:35: error: invalid digit 'a' in decimal constant
struct __declspec (uuid(12345678-9abc-def0-1234-56789abcdef0)) s;
  ^
m3.cpp:1:39: error: use of undeclared identifier 'def0'
struct __declspec (uuid(12345678-9abc-def0-1234-56789abcdef0)) s;
  ^
m3.cpp:1:54: error: invalid digit 'a' in decimal constant
struct __declspec (uuid(12345678-9abc-def0-1234-56789abcdef0)) s;
 ^
3 errors generated.
ksh-3.2$



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

https://reviews.llvm.org/D43576



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

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

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18
+
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+} // namespace ast_matchers

@aaron.ballman:  This matcher seems genuinely useful.  What do you think about 
moving it to ASTMatchers.h? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193260.
ymandel marked 3 inline comments as done.
ymandel added a comment.

Small tweaks in response to comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -0,0 +1,388 @@
+//===- unittest/Tooling/TransformerTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Transformer.h"
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+namespace {
+using ast_matchers::anyOf;
+using ast_matchers::argumentCountIs;
+using ast_matchers::callee;
+using ast_matchers::callExpr;
+using ast_matchers::cxxMemberCallExpr;
+using ast_matchers::cxxMethodDecl;
+using ast_matchers::cxxRecordDecl;
+using ast_matchers::declRefExpr;
+using ast_matchers::expr;
+using ast_matchers::functionDecl;
+using ast_matchers::hasAnyName;
+using ast_matchers::hasArgument;
+using ast_matchers::hasDeclaration;
+using ast_matchers::hasElse;
+using ast_matchers::hasName;
+using ast_matchers::hasType;
+using ast_matchers::ifStmt;
+using ast_matchers::member;
+using ast_matchers::memberExpr;
+using ast_matchers::namedDecl;
+using ast_matchers::on;
+using ast_matchers::pointsTo;
+using ast_matchers::to;
+using ast_matchers::unless;
+
+using llvm::StringRef;
+
+constexpr char KHeaderContents[] = R"cc(
+  struct string {
+string(const char*);
+char* c_str();
+int size();
+  };
+  int strlen(const char*);
+
+  namespace proto {
+  struct PCFProto {
+int foo();
+  };
+  struct ProtoCommandLineFlag : PCFProto {
+PCFProto& GetProto();
+  };
+  }  // namespace proto
+)cc";
+
+static ast_matchers::internal::Matcher
+isOrPointsTo(const clang::ast_matchers::DeclarationMatcher &TypeMatcher) {
+  return anyOf(hasDeclaration(TypeMatcher), pointsTo(TypeMatcher));
+}
+
+static std::string format(StringRef Code) {
+  const std::vector Ranges(1, Range(0, Code.size()));
+  auto Style = format::getLLVMStyle();
+  const auto Replacements = format::reformat(Style, Code, Ranges);
+  auto Formatted = applyAllReplacements(Code, Replacements);
+  if (!Formatted) {
+ADD_FAILURE() << "Could not format code: "
+  << llvm::toString(Formatted.takeError());
+return std::string();
+  }
+  return *Formatted;
+}
+
+static void compareSnippets(StringRef Expected,
+ const llvm::Optional &MaybeActual) {
+  ASSERT_TRUE(MaybeActual) << "Rewrite failed. Expecting: " << Expected;
+  auto Actual = *MaybeActual;
+  std::string HL = "#include \"header.h\"\n";
+  auto I = Actual.find(HL);
+  if (I != std::string::npos)
+Actual.erase(I, HL.size());
+  EXPECT_EQ(format(Expected), format(Actual));
+}
+
+// FIXME: consider separating this class into its own file(s).
+class ClangRefactoringTestBase : public testing::Test {
+protected:
+  void appendToHeader(StringRef S) { FileContents[0].second += S; }
+
+  void addFile(StringRef Filename, StringRef Content) {
+FileContents.emplace_back(Filename, Content);
+  }
+
+  llvm::Optional rewrite(StringRef Input) {
+std::string Code = ("#include \"header.h\"\n" + Input).str();
+auto Factory = newFrontendActionFactory(&MatchFinder);
+if (!runToolOnCodeWithArgs(
+Factory->create(), Code, std::vector(), "input.cc",
+"clang-tool", std::make_shared(),
+FileContents)) {
+  return None;
+}
+auto ChangedCodeOrErr =
+applyAtomicChanges("input.cc", Code, Changes, ApplyChangesSpec());
+if (auto Err = ChangedCodeOrErr.takeError()) {
+  llvm::errs() << "Change failed: " << llvm::toString(std::move(Err))
+   << "\n";
+  return None;
+}
+return *ChangedCodeOrErr;
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+Transformer T(std::move(Rule),
+  [this](const AtomicChange &C) { Changes.push_back(C); });
+T.registerMatchers(&MatchFinder);
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  clang::ast_matchers::MatchFinder MatchFinder;
+  AtomicChanges Changes;
+
+private:
+  FileContentMappings FileContents = {{"header.h", ""}};
+};
+
+class Transf

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thank you for the extremely helpful and detailed review!!




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:110
+  // constructed with the builder class.
+  static constexpr char RootId[] = "___root___";
+};

ilya-biryukov wrote:
> NIT: `llvm::StringLiteral` is a vocabulary type with compile-time size that 
> could be used here.
> Although I don't think it has any actual benefits over char array, so leaving 
> as is also looks good.
Went with StringLiteral -- given that we always wrap RootId in a stringref to 
use it, seems better just to define it that way to begin with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

[PATCH] D60123: [AST] Forbid copy/move of Stmt.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added a reviewer: aaron.ballman.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

Statements and expressions are not supposed to be moved, and trying to do so is 
only going to result in tears. Someone tripped on this a few days ago on the 
mailing list. NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D60123

Files:
  include/clang/AST/Stmt.h


Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }


Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60124: gn build: Add build files for non-framework xpc clangd bits

[Nico Weber via Phabricator via cfe-commits]

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

With this, check-clang-tools unit tests are complete, but the lit tests still 
don't run the one XPC test since this is still missing build files for the xpc 
framework.


https://reviews.llvm.org/D60124

Files:
  llvm/utils/gn/build/sync_source_lists_from_cmake.py
  llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
  llvm/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn
@@ -0,0 +1,15 @@
+import("//llvm/utils/unittest/unittest.gni")
+
+unittest("ClangdXpcTests") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//clang-tools-extra/clangd/xpc:conversions",
+"//llvm/lib/Support",
+"//llvm/lib/Testing/Support",
+  ]
+  include_dirs = [ "//clang-tools-extra/clangd" ]
+  sources = [
+"ConversionTests.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
@@ -1,3 +1,5 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
+
 group("unittests") {
   deps = [
 "clang-apply-replacements:ClangApplyReplacementsTests",
@@ -10,6 +12,8 @@
 "clang-tidy:ClangTidyTests",
 "clangd:ClangdTests",
   ]
-  # FIXME: dep on clangd/xpc:ClangdXpcTests once it exists
+  if (clangd_build_xpc) {
+deps += [ "clangd/xpc:ClangdXpcTests" ]
+  }
   testonly = true
 }
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
@@ -0,0 +1,4 @@
+declare_args() {
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = current_os == "mac"
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
@@ -0,0 +1,25 @@
+static_library("conversions") {
+  output_name = "clangdXpcJsonConversions"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//llvm/lib/Support",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"Conversion.cpp",
+  ]
+}
+
+static_library("transport") {
+  output_name = "clangdXpcTransport"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//llvm/lib/Support",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"XPCTransport.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
@@ -1,10 +1,6 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 
-declare_args() {
-  # Whether to build clangd's XPC components.
-  clangd_build_xpc = false
-}
-
 write_cmake_config("features") {
   # FIXME: Try moving Features.inc.in to tools, seems like a better location.
   input = "../Features.inc.in"
@@ -33,6 +29,12 @@
 "//clang/lib/Tooling/Core",
 "//llvm/lib/Support",
   ]
+  if (clangd_build_xpc) {
+deps += [
+  "//clang-tools-extra/clangd/xpc:conversions",
+  "//clang-tools-extra/clangd/xpc:transport",
+]
+  }
 
   include_dirs = [
 "..",
@@ -43,8 +45,4 @@
   sources = [
 "ClangdMain.cpp",
   ]
-
-  if (clangd_build_xpc) {
-# FIXME: Depend on clangdXpcJsonConversions, clangdXpcTransport
-  }
 }
Index: llvm/utils/gn/build/sync_source_lists_from_cmake.py
===
--- llvm/utils/gn/build/sync_source_lists_from_cmake.py
+++ llvm/utils/gn/build/sync_source_lists_from_cmake.py
@@ -61,8 +61,7 @@
 # Matches e.g. |add_llvm_unittest_with_input_files|.
 unittest_re = re.compile(r'^add_\S+_unittest', re.MULTILINE)
 
-# FIXME: Add 'clang-tools-extra'.
-checked = [ 'clang', 'lld', 'llvm' ]
+checked = [ 'clang', 'clang-tools-extra', 'lld', 'llvm' ]
 for c in checked:
 for root, _, _ in os.walk(os.path.join(c, 'unittests')):
 cmake_file = os

[PATCH] D60123: [AST] Forbid copy/move of Stmt.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Probably `Decl` too?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60123



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

[PATCH] D60123: [AST] Forbid copy/move of Stmt.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D60123#1451303 , @lebedev.ri wrote:

> Probably `Decl` too?


Yep. I though it was already done but after checking it seems I remembered 
incorrectly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60123



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:1
+//===- TokenBuffer.h - store tokens of preprocessed files -*- C++ 
-*-=//
+//

ilya-biryukov wrote:
> sammccall wrote:
> > are you sure TokenBuffer is the central concept in this file, rather than 
> > just the thing with the most code? Token.h might end up being a better name 
> > for users.
> I don't mind changing this to `Token.h`, although I'd personally expect that 
> a file with this name only contains a definition for the token class.
> `Tokens.h` would be a better fit from my POV. WDYT?
Sure, Tokens.h SGTM.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:66
+///macroTokens = {'BAR', '(', '1', ')'}.
+class MacroInvocation {
+public:

I'd suggest a more natural order is token -> tokenbuffer -> macroinvocation. 
Forward declare?

This is because the token buffer exposes token streams, which are probably the 
second-most important concept in the file (after tokens)



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:92
+
+/// A list of tokens obtained by lexing and preprocessing a text buffer and a
+/// set of helpers to allow mapping the tokens after preprocessing to the

ilya-biryukov wrote:
> sammccall wrote:
> > Warning, braindump follows - let's discuss further.
> > We talked a bunch offline about the logical and concrete data model here.
> > 
> > As things stand:
> >  - #includes are not expanded, but will refer to a file ID with its own 
> > buffer: `map` is the whole structure
> >  - no information is captured about other PP interactions (PP directives 
> > that generate no tokens, skipped sections
> >  - the spelled sequence of tokens is not directly available (but expanded + 
> > macro invocations may be enough to reconstruct it)
> > 
> > If we keep this model, I think we should spell both of these out in the 
> > comment.
> > But there's another fairly natural model we could consider:
> > 
> >  - we have a single TokenBuffer with a flat list of all expanded tokens in 
> > the TU, rather than for the file 
> >  - one FileID corresponds to a contiguous range of *transitive* symbols, 
> > these ranges nest
> >  - the TokenBuffer also stores the original tokens as `map > vector>`
> >  - spelled -> expanded token mapping: spelled tokens for a file are 
> > partitioned into ranges (types: literal, include, macro invocation, other 
> > pp directive, pp skipped region). each range maps onto a range of expanded 
> > tokens (empty for e.g. pp directive or skipped region)
> >  - expanded -> spelled token is similar. (It's almost the inverse of the of 
> > the other mapping, we drop the "include" mapping ranges)
> >  - This can naturally handle comments, preprocessor directives, pp skipped 
> > sections, etc - these are in the spelled stream but not the expanded stream.
> > 
> > e.g. for this code
> > ```
> > // foo.cpp
> > int a;
> > #include "foo.h"
> > int b =  FOO(42);
> > // foo.h
> > #define FOO(X) X*X
> > int c;
> > ```
> > 
> > we'd have this buffer:
> > ```
> > expandedTokens = [int a ; int c ; int b = 42 * 42 ;]
> > spelledTokens = {
> >   "foo.cpp" => [int a; # include "foo.h" int b = FOO ( 42 ) ;],
> >   "foo.h" => [# define FOO ( X ) X * X int c ;]
> > }
> > 
> > expandedToSpelling = {
> >   int => int (foo.cpp), type = literal
> >   a => a
> >   ; => ;
> > 
> >   [] => [# define FOO ( X ) X * X](foo.h), type=preprocessor directive
> >   int => int (foo.h)
> >   c => c
> >   ; => ;
> > 
> >   int => int (foo.cpp)
> >   b => b
> >   = => =
> >   [42 * 42] => [FOO ( 42 ) ](foo.cpp), type=macro invocation
> >   ; => ; (foo.cpp)
> > }
> > 
> > spellingToExpanded = {
> >   // foo.cpp
> >   int => int, type=literal
> >   a => a
> >   ; => ;
> >   [# include "foo.h"] => [int c ;], type=include
> >   int => int
> >   b => b
> >   = => =
> >   [FOO ( X )] => [42 * 42], type=macro invocation
> >   ; => ;
> > 
> >   // foo.h
> >   [# define FOO ( X ) X] => [], type=preprocessor directive
> >   int => int
> >   c => c
> >   ; => ;
> > }
> > ```
> > 
> > Various implementation strategies possible here, one obvious one is to use 
> > a flat sorted list, and include a sequence of literal tokens as a single 
> > entry. 
> > 
> > ```
> > struct ExpandedSpellingMapping {
> >   unsigned ExpandedStart, ExpandedEnd;
> >   FileID SpellingFile; // redundant for illustration: can actually derive 
> > from SpellingTokens[SpellingStart].location()
> >   unsigned SpellingStart, SpellingEnd;
> >   enum { LiteralSequence, MacroInvocation, Preprocessor, PPSkipped, 
> > Inclusion } Kind;
> > }
> > vector ExpandedToSpelling; // bsearchable
> > vector> Inclusions; // for spelling 
> > -> expanded mapping. redundant: much can be taken from SourceManager.
> > ```
> > 
> > A delta-encoded representation with chunks for bsearch will likely be much 
> > more compact, if th

[PATCH] D60123: [AST] Forbid copy/move of statements/declarations/types.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 193267.
riccibruno retitled this revision from "[AST] Forbid copy/move of Stmt." to 
"[AST] Forbid copy/move of statements/declarations/types.".
riccibruno edited the summary of this revision.
riccibruno added a reviewer: lebedev.ri.

Repository:
  rC Clang

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

https://reviews.llvm.org/D60123

Files:
  include/clang/AST/DeclBase.h
  include/clang/AST/Stmt.h
  include/clang/AST/Type.h


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1813,7 +1813,9 @@
   friend class ASTWriter;
 
   Type(const Type &) = delete;
+  Type(Type &&) = delete;
   Type &operator=(const Type &) = delete;
+  Type &operator=(Type &&) = delete;
 
   TypeClass getTypeClass() const { return static_cast(TypeBits.TC); 
}
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -386,6 +386,11 @@
 if (StatisticsEnabled) add(DK);
   }
 
+  Decl(const Decl &) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl &) = delete;
+  Decl &operator=(Decl &&) = delete;
+
   virtual ~Decl();
 
   /// Update a potentially out-of-date declaration.


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1813,7 +1813,9 @@
   friend class ASTWriter;
 
   Type(const Type &) = delete;
+  Type(Type &&) = delete;
   Type &operator=(const Type &) = delete;
+  Type &operator=(Type &&) = delete;
 
   TypeClass getTypeClass() const { return static_cast(TypeBits.TC); }
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -386,6 +386,11 @@
 if (StatisticsEnabled) add(DK);
   }
 
+  Decl(const Decl &) = delete;
+  Decl(Decl &&) = delete;
+  Decl &operator=(const Decl &) = delete;
+  Decl &operator=(Decl &&) = delete;
+
   virtual ~Decl();
 
   /// Update a potentially out-of-date declaration.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 193268.
ioeric added a comment.

- Rebase


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59811

Files:
  clangd/ClangdServer.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -15,14 +15,13 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
-using ::testing::_;
-using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
@@ -35,13 +34,18 @@
 
 class TUSchedulerTests : public ::testing::Test {
 protected:
-  ParseInputs getInputs(PathRef File, std::string Contents) {
+  FileUpdateInputs getInputs(PathRef File, std::string Contents) {
 ParseInputs Inputs;
-Inputs.CompileCommand = *CDB.getCompileCommand(File);
 Inputs.FS = buildTestFS(Files, Timestamps);
 Inputs.Contents = std::move(Contents);
 Inputs.Opts = ParseOptions();
-return Inputs;
+FileUpdateInputs UpdateInputs;
+UpdateInputs.InputSansCommand = std::move(Inputs);
+std::string FilePath = File;
+UpdateInputs.GetCompileCommand = [this, FilePath]() {
+  return *CDB.getCompileCommand(FilePath);
+};
+return UpdateInputs;
   }
 
   void updateWithCallback(TUScheduler &S, PathRef File,
@@ -72,7 +76,7 @@
   /// Schedule an update and call \p CB with the diagnostics it produces, if
   /// any. The TUScheduler should be created with captureDiags as a
   /// DiagsCallback for this to work.
-  void updateWithDiags(TUScheduler &S, PathRef File, ParseInputs Inputs,
+  void updateWithDiags(TUScheduler &S, PathRef File, FileUpdateInputs Inputs,
WantDiagnostics WD,
llvm::unique_function)> CB) {
 Path OrigFile = File.str();
@@ -245,7 +249,7 @@
 
 S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
   [&](Expected Pre) {
-ASSERT_TRUE(bool(Pre));
+EXPECT_TRUE((bool)Pre);
 assert(bool(Pre));
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
@@ -397,8 +401,9 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)AST);
-EXPECT_EQ(AST->Inputs.FS, Inputs.FS);
-EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
+EXPECT_EQ(AST->Inputs.FS, Inputs.InputSansCommand.FS);
+EXPECT_EQ(AST->Inputs.Contents,
+  Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalASTReads;
@@ -415,7 +420,7 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)Preamble);
-EXPECT_EQ(Preamble->Contents, Inputs.Contents);
+EXPECT_EQ(Preamble->Contents, Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalPreambleReads;
@@ -504,6 +509,7 @@
   )cpp";
   auto WithEmptyPreamble = R"cpp(int main() {})cpp";
   S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto);
+
   S.runWithPreamble(
   "getNonEmptyPreamble", Foo, TUScheduler::Stale,
   [&](Expected Preamble) {
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -13,8 +13,10 @@
 #include "Matchers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
+#include "Threading.h"
 #include "URI.h"
 #include "clang/Config/config.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Errc.h"
@@ -36,7 +38,6 @@
 namespace {
 
 using ::testing::ElementsAre;
-using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Gt;
 using ::testing::IsEmpty;
@@ -1058,6 +1059,55 @@
   EXPECT_NE(Result, "");
 }
 
+TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  // Returns compile command only when notified.
+  class DelayedCompilationDatabase : public GlobalCompilationDatabase {
+  public:
+DelayedCompilationDatabase(Notification &CanReturnCommand)
+: CanReturnCommand(CanReturnCommand) {}
+
+llvm::Optional
+getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
+  CanReturnCommand.wait();
+  auto FileName = llvm::sys::

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193269.
Tyker added a comment.

fixed the CFG issue is an proper way


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -63,3 +63,23 @@
 // expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
 #endif
 }
+
+//clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a C++2a extension}}
+#endif
+{
+  f(i);
+}
+  else if (i == 2)
+[[likely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'likely' attribute is a C++2a extension}}
+#endif
+return f(i);
+  return i;
+}
Index: clang/test/PCH/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,57 @@
+// Test this without pch.
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s -ast-dump | FileCheck %s
+
+// Test with pch. Use '-ast-dump' to force deserialization of function bodies.
+// RUN: %clang_cc1 -x c++-header -std=c++2a -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -fsyntax-only -verify 

[PATCH] D60123: [AST] Forbid copy/move of statements/declarations/types.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D60123#1451307 , @riccibruno wrote:

> In D60123#1451303 , @lebedev.ri 
> wrote:
>
> > Probably `Decl` too?
>
>
> Yep. I though it was already done but after checking it seems I remembered 
> incorrectly.


Huh, never mind. It seems that at least `ImplicitParamDecl`s are used in a 
`SmallVector`, and this requires the copy-constructor to be usable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60123



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:109
+  /// not supported.
+  virtual llvm::StringRef fixDescription() { return ""; }
+

alexfh wrote:
> Checks can provide different fixes with different semantics depending on the 
> context. It seems incorrect to assume that there can be one reasonable 
> description for all of them.
Yes, as mentioned in the comment, this is the limitation of this approach -- I 
assume we don't have a large number of these checks, most checks are providing 
a  single fix.

Another alternative is to use some heuristic mechanisms to do a translation 
from the `Replacement`, like we do in clangd, 
https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/Diagnostics.cpp#L350.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59932



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

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

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 193277.
ztamas added a comment.

Fixed typo.
Mentioned the default value in release notes.


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

https://reviews.llvm.org/D59870

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

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

[PATCH] D60123: [AST] Forbid copy/move of statements/types.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 193271.
riccibruno retitled this revision from "[AST] Forbid copy/move of 
statements/declarations/types." to "[AST] Forbid copy/move of 
statements/types.".

Repository:
  rC Clang

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

https://reviews.llvm.org/D60123

Files:
  include/clang/AST/Stmt.h
  include/clang/AST/Type.h


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1813,7 +1813,9 @@
   friend class ASTWriter;
 
   Type(const Type &) = delete;
+  Type(Type &&) = delete;
   Type &operator=(const Type &) = delete;
+  Type &operator=(Type &&) = delete;
 
   TypeClass getTypeClass() const { return static_cast(TypeBits.TC); 
}
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1813,7 +1813,9 @@
   friend class ASTWriter;
 
   Type(const Type &) = delete;
+  Type(Type &&) = delete;
   Type &operator=(const Type &) = delete;
+  Type &operator=(Type &&) = delete;
 
   TypeClass getTypeClass() const { return static_cast(TypeBits.TC); }
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1042,6 +1042,11 @@
 return static_cast(StmtBits.sClass);
   }
 
+  Stmt(const Stmt &) = delete;
+  Stmt(Stmt &&) = delete;
+  Stmt &operator=(const Stmt &) = delete;
+  Stmt &operator=(Stmt &&) = delete;
+
   const char *getStmtClassName() const;
 
   bool isOMPStructuredBlock() const { return StmtBits.IsOMPStructuredBlock; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

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

o Lex the code to get the identifiers and put them into a "symbol" index.
o Adds a new completion mode without compilation/sema into code completion 
workflow.
o Make IncludeInserter work even when no compile command is present, by avoiding
inserting non-verbatim headers.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp

Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -90,7 +90,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 for (const auto &Inc : Inclusions)
   Inserter.addExisting(Inc);
 auto Declaring = ToHeaderFile(Original);
@@ -110,7 +110,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo());
 auto Edit = Inserter.insert(VerbatimHeader);
 Action.EndSourceFile();
 return Edit;
@@ -252,6 +252,24 @@
   EXPECT_TRUE(StringRef(Edit->newText).contains(""));
 }
 
+TEST(Headers, NoHeaderSearchInfo) {
+  std::string MainFile = testPath("main.cpp");
+  IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+   /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+
+  auto HeaderPath = testPath("sub/bar.h");
+  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -139,6 +138,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  MockFSProvider FS;
+  Annotations Test(Text);
+  return codeCompleteNoCompile(FilePath, Test.code(), Test.point(),
+   FS.getFileSystem(), Opts);
+}
+
 Symbol withReferences(int N, Symbol S) {
   S.References = N;
   return S;
@@ -2366,6 +2384,33 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+TEST(NoCompileCompletionTest, Basic) {
+  auto Results = completionsNoCompile(R"cpp(
+void func() {
+  int xyz;
+  int abc;
+  ^
+}
+  )cpp");
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(Named("void"), Named("func"), Named("int"),
+   Named("xyz"), Named("abc")));
+}
+
+TEST(NoCompileCompletionTest, WithFilter) {
+  auto Results = completionsNoCompile(R"cpp(
+void func() {
+  int sym1;
+  int sym2;
+  int xyz1;
+  int xyz2;
+  sy^
+}
+  )cpp");
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(Named("sym1"), Named("sym2")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace cla

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Leaving a few first comments, have to run.
Will take another look later, sorry about the delay today (also feel free to 
add more reviewers in case this is urgent).




Comment at: clangd/ClangdServer.cpp:197
   return CB(llvm::make_error());
+if (!IP->Preamble) {
+  vlog("File {0} is not ready for code completion. Enter fallback mode.",

ioeric wrote:
> ilya-biryukov wrote:
> > This way we don't distinguish between the failure to build a preamble and 
> > the fallback mode.
> > Have you considered introducing a different callback instead to clearly 
> > separate two cases for the clients?
> > The code handling those would hardly have any similarities anyway, given 
> > that the nature of those two cases is so different.
> > 
> > Would look something like this:
> > ```
> > /// When \p FallbackAction is not null, it would be called instead of \p 
> > Action in cases when preamble is 
> > /// not yet ready.
> > void runWithPreamble(… Callback Action, Callback 
> > FallbackAction);
> > ```
> > void runWithPreamble(… Callback Action, Callback 
> > FallbackAction);
> This is what I started with in the first iteration. The main problem is that 
> we would need to bind `CB` to both actions; however, `CB` is not 
> copyable/shareable by design, I think. This seems to leave us with the only 
> option to handle everything in the same action. I thought this was fine 
> because an parameter `AllowFallback` seems to be clear as well. I'm open to 
> suggestions ;)
> 
> > This way we don't distinguish between the failure to build a preamble and 
> > the fallback mode.
> I am assuming that a fallback for `runWithPreamble` is either compile command 
> is missing/broken or preamble fails to be built. And in both cases, fallback 
> mode could be useful. Do you think we should have more distinctions made here?
Ah, right, I did not realize that design of `Callback` makes it hard for two 
callbacks to own the same data.
And that's what we technically need for this. I'll have to think about this 
two, maybe there's a better way to express this in the code (`AllowFallback` 
looks like the option with least amount of friction so far).



Comment at: clangd/TUScheduler.cpp:186
 
   std::shared_ptr getPossiblyStalePreamble() const;
+

ioeric wrote:
> ilya-biryukov wrote:
> > Could we put the preamble and compile command into a single function?
> > Getting them in the lock-step would mean they're always consistent, which 
> > is a good thing.
> Could you elaborate the advantages of keeping them consistent?
> 
> They seem to be updated and used relatively independently. Grouping them into 
> one function seems to add more work to the call sites.  And it seems 
> inevitable that we would some inconsistency between the two.
I was originally thinking about consistency between the provided compile 
command and the one used to build the preamble.
Now I realize that they are set at different times, so we can't really make 
them consistent (unless we decide to delay the update of the compile command, 
but I don't see why it's useful).

You approach looks legit, but could you please document in these getters (maybe 
more importantly in the public callback interfaces?) that preamble and compile 
commands might be out of sync (command can be newer) and clients should handle 
that gracefully.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59811



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

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

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas marked 3 inline comments as done.
ztamas added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:33
+
+  Upper limit for the magnitue bits of the loop variable. If it's set the check
+  filters out those catches in which the loop variable's type has more 
magnitude

JonasToth wrote:
> typo `magnitude`
> 
> The commit you reference to the project fixing the "bugs" stated, that the 
> version without conversions is more efficient (by one instruction :)). I 
> think this might be worth a note, that even though the
> code is not a bug in itself it might still be preferable to remove those code 
> locations in the long run.
Typo is fixed

The referenced commit message mentions 32 bit system, where changing integer 
types was a bit faster. I'm not sure how general this is. I'm not familiar with 
the low level implementation of integer comparison, so I would not mention 
something, that I don't actually understand how it works.


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

https://reviews.llvm.org/D59870



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Not sure what the implications of design changes are, so will defer reviewing 
details of tokencollector (which generally looks good, but is of course highly 
coupled to lexer/pp) and range mapping (which I suspect could be simplified, 
but depends heavily on the model).




Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:61
+
+class TokenCollector::Callbacks : public PPCallbacks {
+public:

I'm afraid this really needs a class comment describing what this is supposed 
to do (easy!) and the implementation strategy (hard!)

In particular, it looks like macros like this:
 - we expect to see the tokens making up the macro invocation first (... FOO ( 
42 ))
 - then we see MacroExpands which allows us to recognize the last N tokens are 
a macro invocation. We create a MacroInvocation object, and remember where the 
invocation ends
 - then we see the tokens making up the macro expansion
 - finally, once we see the next token after the invocation, we finalize the 
MacroInvocation.



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:285
+std::tie(File, Offset) = SM.getDecomposedLoc(L);
+if (File != MacroInvocationFile || Offset <= ExpansionEndOffset)
+  return;

is there a problem if we never see another token in that file after the 
expansion?
(or do we see the eof token in this case?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887



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

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 193281.
Anastasia added a comment.

- Use `AggValueSlot` in the constructor call generation to store/retrieve 
address space of 'this'.
- Fixed detecting the address space conversion while performing qualification 
conversion.


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

https://reviews.llvm.org/D59988

Files:
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaInit.cpp
  test/CodeGenCXX/address-space-of-this.cpp

Index: test/CodeGenCXX/address-space-of-this.cpp
===
--- /dev/null
+++ test/CodeGenCXX/address-space-of-this.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -std=c++14 -triple=spir -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++17 -triple=spir -emit-llvm -o - | FileCheck %s
+
+struct MyType {
+  MyType(int i) : i(i) {}
+  int i;
+};
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast (%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7321,9 +7321,19 @@
 ExprResult Sema::PerformQualificationConversion(Expr *E, QualType Ty,
 ExprValueKind VK,
 CheckedConversionKind CCK) {
-  CastKind CK = (Ty.getAddressSpace() != E->getType().getAddressSpace())
-? CK_AddressSpaceConversion
-: CK_NoOp;
+
+  CastKind CK = CK_NoOp;
+
+  if (VK == VK_RValue) {
+auto PointeeTy = Ty->getPointeeType();
+auto ExprPointeeTy = E->getType()->getPointeeType();
+if (!PointeeTy.isNull() &&
+PointeeTy.getAddressSpace() != ExprPointeeTy.getAddressSpace())
+  CK = CK_AddressSpaceConversion;
+  } else if (Ty.getAddressSpace() != E->getType().getAddressSpace()) {
+CK = CK_AddressSpaceConversion;
+  }
+
   return ImpCastExprToType(E, Ty, CK, VK, /*BasePath=*/nullptr, CCK);
 }
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2503,16 +2503,13 @@
 
   void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
   bool ForVirtualBase, bool Delegating,
-  Address This, const CXXConstructExpr *E,
-  AggValueSlot::Overlap_t Overlap,
-  bool NewPointerIsChecked);
+  AggValueSlot ThisAVS, const CXXConstructExpr *E);
 
   void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
   bool ForVirtualBase, bool Delegating,
   Address This, CallArgList &Args,
   AggValueSlot::Overlap_t Overlap,
-  SourceLocation Loc,
-  bool NewPointerIsChecked);
+  SourceLocation Loc, bool NewPointerIsChecked);
 
   /// Emit assumption load for all bases. Requires to be be called only on
   /// most-derived class and not under construction of the object.
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -617,12 +617,10 @@
 
  case CXXConstructExpr::CK_NonVirtualBase:
   Type = Ctor_Base;
-}
+ }
 
-// Call the constructor.
-EmitCXXConstructorCall(CD, Type, ForVirtualBase, Delegating,
-   Dest.getAddress(), E, Dest.mayOverlap(),
-   Dest.isSanitizerChecked());
+ // Call the constructor.
+ EmitCXXConstructorCall(CD, Type, ForVirtualBase, Delegating, Dest, E);
   }
 }
 
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1970,10 +1970,14 @@
   pushRegularPartialArrayCleanup(arrayBegin, cur, type, eltAlignment,
  *destroyer);
 }
-
+auto currAVS = AggValueSlot::forAddr(
+curAddr, type.getQualifiers(), AggValueSlot::IsDestructed,
+AggValueSlot::DoesNotNeedGCBarriers, AggValueSlot::IsNotAliased,
+AggValueSlot::DoesNotOverlap, AggValueSlot::IsNotZeroed,
+NewPointerIsChecked ? AggValueSlot::IsSanitizerChecked
+: AggValueSlot::IsNotSanitizerChecked);
 EmitCXXConstructorCall(ctor, Ctor_Complete, /*ForVirtualBase=*/false,
-   /*Delegating=*/false, curAddr, E,
-   AggValueSlot::DoesNotOverlap, NewPointerIsChecked);
+   /*Delegating=*/false, currAVS, E);
   }
 
   // Go to

[PATCH] D60124: gn build: Add build files for non-framework xpc clangd bits

[Mirko Bonadei via Phabricator via cfe-commits]

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

LGTM, just a couple of comments.




Comment at: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn:17
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",

Missing ":conversions" in deps.



Comment at: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni:3
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = current_os == "mac"
+}

Is it ok to keep this ` = current_os == "mac"`, from the CL description I had 
the feeling that xpc tests were still disabled.


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

https://reviews.llvm.org/D60124



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193284.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -63,3 +63,23 @@
 // expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
 #endif
 }
+
+//clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a C++2a extension}}
+#endif
+{
+  f(i);
+}
+  else if (i == 2)
+[[likely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'likely' attribute is a C++2a extension}}
+#endif
+return f(i);
+  return i;
+}
Index: clang/test/PCH/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,57 @@
+// Test this without pch.
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s -ast-dump | FileCheck %s
+
+// Test with pch. Use '-ast-dump' to force deserialization of function bodies.
+// RUN: %clang_cc1 -x c++-header -std=c++2a -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -fsyntax-only -verify %s
+// -ast-dump-all | FileCheck %s
+
+// expected-no-diagnost

[PATCH] D60128: Add const children accessors to all nodes.

[Nicolas Manichon via Phabricator via cfe-commits]

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

Add const children accessors to all AST nodes where non const accessors
exist.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60128

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/StmtObjC.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/Stmt.cpp

Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -1254,6 +1254,10 @@
   return child_range(getStoredStmts(), getStoredStmts() + NumCaptures);
 }
 
+Stmt::const_child_range CapturedStmt::children() const {
+  return const_child_range(getStoredStmts(), getStoredStmts() + NumCaptures);
+}
+
 CapturedDecl *CapturedStmt::getCapturedDecl() {
   return CapDeclAndKind.getPointer();
 }
Index: clang/lib/AST/ExprObjC.cpp
===
--- clang/lib/AST/ExprObjC.cpp
+++ clang/lib/AST/ExprObjC.cpp
@@ -377,6 +377,11 @@
  reinterpret_cast(getArgs() + getNumArgs()));
 }
 
+Stmt::const_child_range ObjCMessageExpr::children() const {
+  auto Children = const_cast(this)->children();
+  return const_child_range(Children.begin(), Children.end());
+}
+
 StringRef ObjCBridgedCastExpr::getBridgeKindName() const {
   switch (getBridgeKind()) {
   case OBC_Bridge:
Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -256,6 +256,14 @@
 return child_range(ChildStorage, ChildStorage + 1);
   }
 
+  const_child_range children() const {
+if (!hasAssociatedStmt())
+  return const_child_range(const_child_iterator(), const_child_iterator());
+Stmt **ChildStorage = reinterpret_cast(
+const_cast(this)->getClauses().end());
+return const_child_range(ChildStorage, ChildStorage + 1);
+  }
+
   ArrayRef clauses() { return getClauses(); }
 
   ArrayRef clauses() const {
Index: clang/include/clang/AST/StmtObjC.h
===
--- clang/include/clang/AST/StmtObjC.h
+++ clang/include/clang/AST/StmtObjC.h
@@ -67,6 +67,10 @@
   child_range children() {
 return child_range(&SubExprs[0], &SubExprs[END_EXPR]);
   }
+
+  const_child_range children() const {
+return const_child_range(&SubExprs[0], &SubExprs[END_EXPR]);
+  }
 };
 
 /// Represents Objective-C's \@catch statement.
@@ -113,6 +117,10 @@
   }
 
   child_range children() { return child_range(&Body, &Body + 1); }
+
+  const_child_range children() const {
+return const_child_range(&Body, &Body + 1);
+  }
 };
 
 /// Represents Objective-C's \@finally statement
@@ -147,6 +155,10 @@
   child_range children() {
 return child_range(&AtFinallyStmt, &AtFinallyStmt+1);
   }
+
+  const_child_range children() const {
+return const_child_range(&AtFinallyStmt, &AtFinallyStmt + 1);
+  }
 };
 
 /// Represents Objective-C's \@try ... \@catch ... \@finally statement.
@@ -248,6 +260,10 @@
 return child_range(getStmts(),
getStmts() + 1 + NumCatchStmts + HasFinally);
   }
+
+  const_child_range children() const {
+return const_child_range(const_cast(this)->children());
+  }
 };
 
 /// Represents Objective-C's \@synchronized statement.
@@ -306,6 +322,10 @@
   child_range children() {
 return child_range(&SubStmts[0], &SubStmts[0]+END_EXPR);
   }
+
+  const_child_range children() const {
+return const_child_range(&SubStmts[0], &SubStmts[0] + END_EXPR);
+  }
 };
 
 /// Represents Objective-C's \@throw statement.
@@ -338,6 +358,10 @@
   }
 
   child_range children() { return child_range(&Throw, &Throw+1); }
+
+  const_child_range children() const {
+return const_child_range(&Throw, &Throw + 1);
+  }
 };
 
 /// Represents Objective-C's \@autoreleasepool Statement
@@ -369,6 +393,10 @@
   }
 
   child_range children() { return child_range(&SubStmt, &SubStmt + 1); }
+
+  const_child_range children() const {
+return const_child_range(&SubStmt, &SubStmt + 1);
+  }
 };
 
 }  // end namespace clang
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -56,6 +56,10 @@
 
   child_range children() { return child_range(&HandlerBlock, &HandlerBlock+1); }
 
+  const_child_range children() const {
+return const_child_range(&HandlerBlock, &HandlerBlock + 1);
+  }
+
   friend class ASTStmtReader;
 };
 
@@ -114,6 +118,10 @@
   child_range children() {
 return child_range(getStmts(), getStmts() + ge

[PATCH] D60029: Add const children() accessors to all AST nodes.

[Nicolas Manichon via Phabricator via cfe-commits]

nicolas updated this revision to Diff 193288.
nicolas retitled this revision from "Add const children() accessors to Stmts" 
to "Add const children() accessors to all AST nodes.".
nicolas edited the summary of this revision.
nicolas added a comment.

Added children() const to all AST nodes.


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

https://reviews.llvm.org/D60029

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/StmtObjC.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/Stmt.cpp

Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -1254,6 +1254,10 @@
   return child_range(getStoredStmts(), getStoredStmts() + NumCaptures);
 }
 
+Stmt::const_child_range CapturedStmt::children() const {
+  return const_child_range(getStoredStmts(), getStoredStmts() + NumCaptures);
+}
+
 CapturedDecl *CapturedStmt::getCapturedDecl() {
   return CapDeclAndKind.getPointer();
 }
Index: clang/lib/AST/ExprObjC.cpp
===
--- clang/lib/AST/ExprObjC.cpp
+++ clang/lib/AST/ExprObjC.cpp
@@ -377,6 +377,11 @@
  reinterpret_cast(getArgs() + getNumArgs()));
 }
 
+Stmt::const_child_range ObjCMessageExpr::children() const {
+  auto Children = const_cast(this)->children();
+  return const_child_range(Children.begin(), Children.end());
+}
+
 StringRef ObjCBridgedCastExpr::getBridgeKindName() const {
   switch (getBridgeKind()) {
   case OBC_Bridge:
Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -256,6 +256,14 @@
 return child_range(ChildStorage, ChildStorage + 1);
   }
 
+  const_child_range children() const {
+if (!hasAssociatedStmt())
+  return const_child_range(const_child_iterator(), const_child_iterator());
+Stmt **ChildStorage = reinterpret_cast(
+const_cast(this)->getClauses().end());
+return const_child_range(ChildStorage, ChildStorage + 1);
+  }
+
   ArrayRef clauses() { return getClauses(); }
 
   ArrayRef clauses() const {
Index: clang/include/clang/AST/StmtObjC.h
===
--- clang/include/clang/AST/StmtObjC.h
+++ clang/include/clang/AST/StmtObjC.h
@@ -67,6 +67,10 @@
   child_range children() {
 return child_range(&SubExprs[0], &SubExprs[END_EXPR]);
   }
+
+  const_child_range children() const {
+return const_child_range(&SubExprs[0], &SubExprs[END_EXPR]);
+  }
 };
 
 /// Represents Objective-C's \@catch statement.
@@ -113,6 +117,10 @@
   }
 
   child_range children() { return child_range(&Body, &Body + 1); }
+
+  const_child_range children() const {
+return const_child_range(&Body, &Body + 1);
+  }
 };
 
 /// Represents Objective-C's \@finally statement
@@ -147,6 +155,10 @@
   child_range children() {
 return child_range(&AtFinallyStmt, &AtFinallyStmt+1);
   }
+
+  const_child_range children() const {
+return const_child_range(&AtFinallyStmt, &AtFinallyStmt + 1);
+  }
 };
 
 /// Represents Objective-C's \@try ... \@catch ... \@finally statement.
@@ -248,6 +260,10 @@
 return child_range(getStmts(),
getStmts() + 1 + NumCatchStmts + HasFinally);
   }
+
+  const_child_range children() const {
+return const_child_range(const_cast(this)->children());
+  }
 };
 
 /// Represents Objective-C's \@synchronized statement.
@@ -306,6 +322,10 @@
   child_range children() {
 return child_range(&SubStmts[0], &SubStmts[0]+END_EXPR);
   }
+
+  const_child_range children() const {
+return const_child_range(&SubStmts[0], &SubStmts[0] + END_EXPR);
+  }
 };
 
 /// Represents Objective-C's \@throw statement.
@@ -338,6 +358,10 @@
   }
 
   child_range children() { return child_range(&Throw, &Throw+1); }
+
+  const_child_range children() const {
+return const_child_range(&Throw, &Throw + 1);
+  }
 };
 
 /// Represents Objective-C's \@autoreleasepool Statement
@@ -369,6 +393,10 @@
   }
 
   child_range children() { return child_range(&SubStmt, &SubStmt + 1); }
+
+  const_child_range children() const {
+return const_child_range(&SubStmt, &SubStmt + 1);
+  }
 };
 
 }  // end namespace clang
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -56,6 +56,10 @@
 
   child_range children() { return child_range(&HandlerBlock, &HandlerBlock+1); }
 
+  const_child_range children() const {
+return const_child_range(&HandlerBlock, &HandlerBlock + 1);
+  }
+

[PATCH] D60124: gn build: Add build files for non-framework xpc clangd bits

[Nico Weber via Phabricator via cfe-commits]

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



Comment at: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn:17
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",

mbonadei wrote:
> Missing ":conversions" in deps.
Thanks! Done.



Comment at: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni:3
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = current_os == "mac"
+}

mbonadei wrote:
> Is it ok to keep this ` = current_os == "mac"`, from the CL description I had 
> the feeling that xpc tests were still disabled.
Yes, because clang-tools/extra/test/BUILD.gn still has 
`CLANGD_BUILD_XPC_SUPPORT=0`, so lit thinks xpc support isn't available and 
doesn't run the xpc lit tests. And the xpc unit tests pass, so they run after 
this change.


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

https://reviews.llvm.org/D60124



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

[PATCH] D60124: gn build: Add build files for non-framework xpc clangd bits

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357476: gn build: Add build files for non-framework xpc 
clangd bits (authored by nico, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60124?vs=193264&id=193290#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60124

Files:
  llvm/trunk/utils/gn/build/sync_source_lists_from_cmake.py
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
  llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn

Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
@@ -1,10 +1,6 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 
-declare_args() {
-  # Whether to build clangd's XPC components.
-  clangd_build_xpc = false
-}
-
 write_cmake_config("features") {
   # FIXME: Try moving Features.inc.in to tools, seems like a better location.
   input = "../Features.inc.in"
@@ -33,6 +29,12 @@
 "//clang/lib/Tooling/Core",
 "//llvm/lib/Support",
   ]
+  if (clangd_build_xpc) {
+deps += [
+  "//clang-tools-extra/clangd/xpc:conversions",
+  "//clang-tools-extra/clangd/xpc:transport",
+]
+  }
 
   include_dirs = [
 "..",
@@ -43,8 +45,4 @@
   sources = [
 "ClangdMain.cpp",
   ]
-
-  if (clangd_build_xpc) {
-# FIXME: Depend on clangdXpcJsonConversions, clangdXpcTransport
-  }
 }
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/enable.gni
@@ -0,0 +1,4 @@
+declare_args() {
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = current_os == "mac"
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/xpc/BUILD.gn
@@ -0,0 +1,26 @@
+static_library("conversions") {
+  output_name = "clangdXpcJsonConversions"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//llvm/lib/Support",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"Conversion.cpp",
+  ]
+}
+
+static_library("transport") {
+  output_name = "clangdXpcTransport"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+":conversions",
+"//clang-tools-extra/clangd",
+"//llvm/lib/Support",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"XPCTransport.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/clangd/xpc/BUILD.gn
@@ -0,0 +1,15 @@
+import("//llvm/utils/unittest/unittest.gni")
+
+unittest("ClangdXpcTests") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//clang-tools-extra/clangd/xpc:conversions",
+"//llvm/lib/Support",
+"//llvm/lib/Testing/Support",
+  ]
+  include_dirs = [ "//clang-tools-extra/clangd" ]
+  sources = [
+"ConversionTests.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/unittests/BUILD.gn
@@ -1,3 +1,5 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
+
 group("unittests") {
   deps = [
 "clang-apply-replacements:ClangApplyReplacementsTests",
@@ -10,6 +12,8 @@
 "clang-tidy:ClangTidyTests",
 "clangd:ClangdTests",
   ]
-  # FIXME: dep on clangd/xpc:ClangdXpcTests once it exists
+  if (clangd_build_xpc) {
+deps += [ "clangd/xpc:ClangdXpcTests" ]
+  }
   testonly = true
 }
Index: llvm/trunk/utils/gn/build/sync_source_lists_from_cmake.py
===
--- llvm/trunk/utils/gn/build/sync_source_lists_from_cmake.py
+++ llvm/trunk/utils/gn/build/sync_source_lists_from_cmake.py
@@ -61,8 +61,7 @@
 # Matches e.g. |add_llvm_unittest_with_input_files|.

[PATCH] D60101: [Sema] Fix a use-after-deallocate of a ParsedAttr

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from some minor nits. Thanks for this!




Comment at: clang/include/clang/Sema/ParsedAttr.h:896
 
+  void takeOneFrom(ParsedAttributes &attrs, ParsedAttr *attr) {
+attrs.getPool().remove(attr);

How about `Attrs` and `PA` to fit with usual naming conventions?



Comment at: clang/test/SemaObjC/arc-property-decl-attrs.m:291
+
+id i1, __weak i2, i3;

Can you add a comment here that explains what this test is intending to cover?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60101



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

[PATCH] D60123: [AST] Forbid copy/move of statements/types.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D60123



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

[PATCH] D60130: gn build: Add build files for clangd xpc framework code

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added reviewers: mbonadei, sdefresne.
Herald added subscribers: jdoerfert, kadircet, arphaman, jkorous, MaskRay, 
ioeric, ilya-biryukov, mgorny.
Herald added a project: LLVM.
thakis edited the summary of this revision.

This is a bit of a larger change since this is the first (and as far as I can 
tell only) place where the LLVM build produces macOS framework bundles.

GN has some built-in support for this, so use that. `gn help create_bundle` has 
a terse description (but it's a bit outdated: `deps` must be `public_deps` and 
the conditionals in the example in the help aren't quite right on non-iOS).

We need a new 'copy_bundle_data' tool, and since we copy the clangd.xpc bundle 
as bundle_data into ClangdXPC.framework it needs to be able to handle 
directories in addition to files.

GN also insists we have a compile_xcassets tool even though it's not used. I 
just made that run `false`.

Despite GN's support for bundles, we still need to manually create the expected 
symlink structure in the .framework bundle. Since this code never runs on 
Windows, it's safe to create the symlinks before the symlink targets exist, so 
we can just make the bundle depend on the steps that create the symlinks. For 
this to work, change the symlink script to create the symlink's containing 
directory if it doesn't yet exist.

I locally verified that CMake and GN build create the same bundle structure. (I 
noticed that both builds set LC_ID_DYLIB to the pre-copy libClangdXPCLib.dylib 
name, but that seems to not cause any issues and it happens in the CMake build 
too.)

(Also add an error message to clangd-xpc-test-client for when loading the dylib 
fails – this was useful while locally debugging this.)


https://reviews.llvm.org/D60130

Files:
  clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
  clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
  llvm/utils/gn/build/symlink_or_copy.py
  llvm/utils/gn/build/toolchain/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
@@ -1,3 +1,4 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
 import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
 import("//llvm/triples.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
@@ -35,9 +36,14 @@
 "LLVM_LIT_TOOLS_DIR=",  # Intentionally empty, matches cmake build.
 "LLVM_TOOLS_DIR=" + rebase_path("$root_out_dir/bin"),
 "PYTHON_EXECUTABLE=$python_path",
-"CLANGD_BUILD_XPC_SUPPORT=0",  # FIXME
   ]
 
+  if (clangd_build_xpc) {
+extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=1" ]
+  } else {
+extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=0" ]
+  }
+
   if (clang_enable_static_analyzer) {
 extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=1" ]
   } else {
@@ -83,6 +89,10 @@
 "//llvm/utils/llvm-lit",
 "//llvm/utils/not",
   ]
+  if (clangd_build_xpc) {
+deps +=
+[ "//clang-tools-extra/clangd/xpc/test-client:clangd-xpc-test-client" ]
+  }
 
   # FIXME: dep on dexp once it exist
   testonly = true
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
@@ -0,0 +1,20 @@
+executable("clangd-xpc-test-client") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//clang-tools-extra/clangd/xpc:conversions",
+"//clang-tools-extra/clangd/xpc/framework:ClangdXPC.framework",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Sema",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+
+  include_dirs = [ "../.." ]
+  sources = [
+"ClangdXPCTestClient.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
@@ -0,0 +1,154 @@
+import("//llvm/utils/gn/build/symlink_or_copy.gni")
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+
+# create_clangd_xpc_framework() in cmake creates both the ClangdXPC.framework
+# bundle and the clangd.xpc bundle within it in a single action.
+# Since GN has some native support for macOS bundles, it's more natural
+# to have one create_bundle() each for both ClangdXPC.framework and clangd.xpc.
+# See `llvm/utils/gn/gn.py help create_bundle` and ../cmake/modules.
+
+#

[PATCH] D60099: [CodeGen] Fix a regression by emitting lambda expressions in EmitLValue

[John McCall via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D60099



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

[PATCH] D60120: check-clang-tools: Actually build and run XPC test

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(I noticed this while working on D60130 )


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

https://reviews.llvm.org/D60120



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

r357480 - [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

[Pierre Gousseau via cfe-commits]

Author: pgousseau
Date: Tue Apr  2 08:20:26 2019
New Revision: 357480

URL: http://llvm.org/viewvc/llvm-project?rev=357480&view=rev
Log:
[Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

Can be safely enabled on PS4.

Reviewed By: probinson

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=357480&r1=357479&r2=357480&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Tue Apr  2 08:20:26 2019
@@ -768,6 +768,7 @@ SanitizerArgs::SanitizerArgs(const ToolC
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=357480&r1=357479&r2=357480&view=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Apr  2 08:20:26 2019
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address 
-fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: 
-cc1{{.*}}-fsanitize-address-globals-dead-stripping
 


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

[PATCH] D59815: [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

[pierre gousseau via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357480: [Driver] Enable 
-fsanitize-address-globals-dead-stripping by default on PS4. (authored by 
pgousseau, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D59815

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize.c


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -768,6 +768,7 @@
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address 
-fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: 
-cc1{{.*}}-fsanitize-address-globals-dead-stripping
 


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -768,6 +768,7 @@
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 193306.
ymandel added a comment.

Remove dependency on NodeIds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -0,0 +1,389 @@
+//===- unittest/Tooling/TransformerTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Transformer.h"
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+namespace {
+using ast_matchers::anyOf;
+using ast_matchers::argumentCountIs;
+using ast_matchers::callee;
+using ast_matchers::callExpr;
+using ast_matchers::cxxMemberCallExpr;
+using ast_matchers::cxxMethodDecl;
+using ast_matchers::cxxRecordDecl;
+using ast_matchers::declRefExpr;
+using ast_matchers::expr;
+using ast_matchers::functionDecl;
+using ast_matchers::hasAnyName;
+using ast_matchers::hasArgument;
+using ast_matchers::hasDeclaration;
+using ast_matchers::hasElse;
+using ast_matchers::hasName;
+using ast_matchers::hasType;
+using ast_matchers::ifStmt;
+using ast_matchers::member;
+using ast_matchers::memberExpr;
+using ast_matchers::namedDecl;
+using ast_matchers::on;
+using ast_matchers::pointsTo;
+using ast_matchers::to;
+using ast_matchers::unless;
+
+using llvm::StringRef;
+
+constexpr char KHeaderContents[] = R"cc(
+  struct string {
+string(const char*);
+char* c_str();
+int size();
+  };
+  int strlen(const char*);
+
+  namespace proto {
+  struct PCFProto {
+int foo();
+  };
+  struct ProtoCommandLineFlag : PCFProto {
+PCFProto& GetProto();
+  };
+  }  // namespace proto
+)cc";
+
+static ast_matchers::internal::Matcher
+isOrPointsTo(const clang::ast_matchers::DeclarationMatcher &TypeMatcher) {
+  return anyOf(hasDeclaration(TypeMatcher), pointsTo(TypeMatcher));
+}
+
+static std::string format(StringRef Code) {
+  const std::vector Ranges(1, Range(0, Code.size()));
+  auto Style = format::getLLVMStyle();
+  const auto Replacements = format::reformat(Style, Code, Ranges);
+  auto Formatted = applyAllReplacements(Code, Replacements);
+  if (!Formatted) {
+ADD_FAILURE() << "Could not format code: "
+  << llvm::toString(Formatted.takeError());
+return std::string();
+  }
+  return *Formatted;
+}
+
+static void compareSnippets(StringRef Expected,
+ const llvm::Optional &MaybeActual) {
+  ASSERT_TRUE(MaybeActual) << "Rewrite failed. Expecting: " << Expected;
+  auto Actual = *MaybeActual;
+  std::string HL = "#include \"header.h\"\n";
+  auto I = Actual.find(HL);
+  if (I != std::string::npos)
+Actual.erase(I, HL.size());
+  EXPECT_EQ(format(Expected), format(Actual));
+}
+
+// FIXME: consider separating this class into its own file(s).
+class ClangRefactoringTestBase : public testing::Test {
+protected:
+  void appendToHeader(StringRef S) { FileContents[0].second += S; }
+
+  void addFile(StringRef Filename, StringRef Content) {
+FileContents.emplace_back(Filename, Content);
+  }
+
+  llvm::Optional rewrite(StringRef Input) {
+std::string Code = ("#include \"header.h\"\n" + Input).str();
+auto Factory = newFrontendActionFactory(&MatchFinder);
+if (!runToolOnCodeWithArgs(
+Factory->create(), Code, std::vector(), "input.cc",
+"clang-tool", std::make_shared(),
+FileContents)) {
+  return None;
+}
+auto ChangedCodeOrErr =
+applyAtomicChanges("input.cc", Code, Changes, ApplyChangesSpec());
+if (auto Err = ChangedCodeOrErr.takeError()) {
+  llvm::errs() << "Change failed: " << llvm::toString(std::move(Err))
+   << "\n";
+  return None;
+}
+return *ChangedCodeOrErr;
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+Transformer T(std::move(Rule),
+  [this](const AtomicChange &C) { Changes.push_back(C); });
+T.registerMatchers(&MatchFinder);
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  clang::ast_matchers::MatchFinder MatchFinder;
+  AtomicChanges Changes;
+
+private:
+  FileContentMappings FileContents = {{"header.h", ""}};
+};
+
+class TransformerTest : public ClangRefactoringTestBase {
+pro

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

I've removed the dependency on NodeIds given our (off thread) discussions 
regarding their overall value and appropriateness in clang::tooling vs 
clang::ast_matchers. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

r357488 - [OPENMP]Fix mapping of the pointers captured by reference.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr  2 09:03:40 2019
New Revision: 357488

URL: http://llvm.org/viewvc/llvm-project?rev=357488&view=rev
Log:
[OPENMP]Fix mapping of the pointers captured by reference.

If the pointer is captured by reference, it must be mapped as
_PTR_AND_OBJ kind of mapping to correctly translate the pointer address
on the device.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=357488&r1=357487&r2=357488&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Apr  2 09:03:40 2019
@@ -7347,6 +7347,9 @@ private:
   Cap.getCaptureKind() == CapturedStmt::VCK_ByRef)
 return MappableExprsHandler::OMP_MAP_ALWAYS |
MappableExprsHandler::OMP_MAP_TO;
+  if (Cap.getCapturedVar()->getType()->isAnyPointerType())
+return MappableExprsHandler::OMP_MAP_TO |
+   MappableExprsHandler::OMP_MAP_PTR_AND_OBJ;
   return MappableExprsHandler::OMP_MAP_PRIVATE |
  MappableExprsHandler::OMP_MAP_TO;
 }
@@ -7992,14 +7995,20 @@ public:
 CGF.Builder.CreateMemCpy(
 CGF.MakeNaturalAlignAddrLValue(Addr, ElementType).getAddress(),
 Address(CV, CGF.getContext().getTypeAlignInChars(ElementType)),
-CurSizes.back(),
-/*isVolatile=*/false);
+CurSizes.back(), /*isVolatile=*/false);
 // Use new global variable as the base pointers.
 CurBasePointers.push_back(Addr);
 CurPointers.push_back(Addr);
   } else {
 CurBasePointers.push_back(CV);
-CurPointers.push_back(CV);
+if (FirstPrivateDecls.count(VD) && ElementType->isAnyPointerType()) {
+  Address PtrAddr = CGF.EmitLoadOfReference(CGF.MakeAddrLValue(
+  CV, ElementType, CGF.getContext().getDeclAlign(VD),
+  AlignmentSource::Decl));
+  CurPointers.push_back(PtrAddr.getPointer());
+} else {
+  CurPointers.push_back(CV);
+}
   }
 }
 // Every default map produces a single argument which is a target 
parameter.

Modified: cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp?rev=357488&r1=357487&r2=357488&view=diff
==
--- cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp Tue Apr  2 09:03:40 
2019
@@ -53,8 +53,8 @@ struct TT {
 // TCHECK-DAG:  [[S1:%.+]] = type { double }
 
 // CHECK-DAG:  [[FP_E:@__omp_offloading_firstprivate_.+_e_l76]] = internal 
global [[TTII]] zeroinitializer
-// CHECK-DAG:  [[SIZET:@.+]] = private unnamed_addr constant [1 x i{{32|64}}] 
[i[[SZ:32|64]] 4]
-// CHECK-DAG:  [[MAPT:@.+]] = private unnamed_addr constant [1 x i64] [i64 800]
+// CHECK-DAG:  [[SIZET:@.+]] = private unnamed_addr constant [2 x i{{32|64}}] 
[i[[SZ:32|64]] 4, i{{64|32}} {{8|4}}]
+// CHECK-DAG:  [[MAPT:@.+]] = private unnamed_addr constant [2 x i64] [i64 
800, i64 561]
 // CHECK-DAG:  [[MAPT2:@.+]] = private unnamed_addr constant [9 x i64] [i64 
800, i64 673, i64 288, i64 673, i64 673, i64 288, i64 288, i64 673, i64 673]
 // CHECK-DAG:  [[SIZET3:@.+]] = private unnamed_addr constant [2 x i{{32|64}}] 
[i{{32|64}} 0, i{{32|64}} 8]
 // CHECK-DAG:  [[MAPT3:@.+]] = private unnamed_addr constant [2 x i64] [i64 
544, i64 549]
@@ -74,8 +74,9 @@ int foo(int n, double *ptr) {
   double cn[5][n];
   TT d;
   const TT e = {n, n};
+  int *p __attribute__ ((aligned (64))) = &a;
 
-#pragma omp target firstprivate(a)
+#pragma omp target firstprivate(a, p)
   {
   }
 
@@ -88,9 +89,10 @@ int foo(int n, double *ptr) {
   // CHECK:  [[SSTACK:%.+]] = alloca i8*,
   // CHECK:  [[C:%.+]] = alloca [5 x [10 x double]],
   // CHECK:  [[D:%.+]] = alloca [[TT]],
+  // CHECK:  [[P:%.+]] = alloca i32*, align 64
   // CHECK:  [[ACAST:%.+]] = alloca i{{[0-9]+}},
-  // CHECK:  [[BASE_PTR_ARR:%.+]] = alloca [1 x i8*],
-  // CHECK:  [[PTR_ARR:%.+]] = alloca [1 x i8*],
+  // CHECK:  [[BASE_PTR_ARR:%.+]] = alloca [2 x i8*],
+  // CHECK:  [[PTR_ARR:%.+]] = alloca [2 x i8*],
   // CHECK:  [[A2CAST:%.+]] = alloca i{{[0-9]+}},
   // CHECK:  [[BASE_PTR_ARR2:%.+]] = alloca [9 x i8*],
   // CHECK:  [[PTR_ARR2:%.+]] = alloca [9 x i8*],
@@ -113,20 +115,30 @@ int foo(int n, double *ptr) {
   // CHECK-64:  store i{{[0-9]+}} [[AVAL]], i{{[0-9]+}}* [[CONV]],
   // CHECK-32:  store i{{[0-9]+}} [[AVAL]], i{{[0-9]+}}* [[ACAST]],
   // CHECK:  [[ACAST_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[ACAST]],
-  // CHECK:  [[BASE_PTR_GEP:%.+]] = getelementptr inbounds [1 x i8*], [1 x 
i8*]* [[BASE_PTR_ARR]], i{{

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

FYI: a new ACLE version has been published, please find it here:   
https://developer.arm.com/architectures/system-architectures/software-standards/acle

The "Neon Intrinsics" section contains these new intrinsics.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53633



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

[PATCH] D60130: gn build: Add build files for clangd xpc framework code

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 193313.
thakis added a comment.

Use {{source_file_part}} in bundle_data where possible.


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

https://reviews.llvm.org/D60130

Files:
  clang-tools-extra/clangd/xpc/framework/CMakeLists.txt
  clang-tools-extra/clangd/xpc/test-client/ClangdXPCTestClient.cpp
  llvm/utils/gn/build/symlink_or_copy.py
  llvm/utils/gn/build/toolchain/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
@@ -1,3 +1,4 @@
+import("//clang-tools-extra/clangd/xpc/enable.gni")
 import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
 import("//llvm/triples.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
@@ -35,9 +36,14 @@
 "LLVM_LIT_TOOLS_DIR=",  # Intentionally empty, matches cmake build.
 "LLVM_TOOLS_DIR=" + rebase_path("$root_out_dir/bin"),
 "PYTHON_EXECUTABLE=$python_path",
-"CLANGD_BUILD_XPC_SUPPORT=0",  # FIXME
   ]
 
+  if (clangd_build_xpc) {
+extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=1" ]
+  } else {
+extra_values += [ "CLANGD_BUILD_XPC_SUPPORT=0" ]
+  }
+
   if (clang_enable_static_analyzer) {
 extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=1" ]
   } else {
@@ -83,6 +89,10 @@
 "//llvm/utils/llvm-lit",
 "//llvm/utils/not",
   ]
+  if (clangd_build_xpc) {
+deps +=
+[ "//clang-tools-extra/clangd/xpc/test-client:clangd-xpc-test-client" ]
+  }
 
   # FIXME: dep on dexp once it exist
   testonly = true
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/test-client/BUILD.gn
@@ -0,0 +1,20 @@
+executable("clangd-xpc-test-client") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clangd",
+"//clang-tools-extra/clangd/xpc:conversions",
+"//clang-tools-extra/clangd/xpc/framework:ClangdXPC.framework",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Sema",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+
+  include_dirs = [ "../.." ]
+  sources = [
+"ClangdXPCTestClient.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clangd/xpc/framework/BUILD.gn
@@ -0,0 +1,154 @@
+import("//llvm/utils/gn/build/symlink_or_copy.gni")
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+
+# create_clangd_xpc_framework() in cmake creates both the ClangdXPC.framework
+# bundle and the clangd.xpc bundle within it in a single action.
+# Since GN has some native support for macOS bundles, it's more natural
+# to have one create_bundle() each for both ClangdXPC.framework and clangd.xpc.
+# See `llvm/utils/gn/gn.py help create_bundle` and ../cmake/modules.
+
+##
+# clangd.xpc bundle
+
+write_cmake_config("XPCServiceInfo.plist") {
+  input = "../cmake/XPCServiceInfo.plist.in"
+  output = "$target_gen_dir/XPCServiceInfo.plist"
+  service_name = "clangd"
+  values = [
+"CLANGD_XPC_SERVICE_NAME=$service_name",
+"CLANGD_XPC_SERVICE_BUNDLE_NAME=org.llvm.$service_name",
+  ]
+}
+
+bundle_data("clangxpc_bundle_xpc_service_info_plist") {
+  public_deps = [
+":XPCServiceInfo.plist",
+  ]
+  sources = [
+"$target_gen_dir/XPCServiceInfo.plist",
+  ]
+  outputs = [
+"{{bundle_contents_dir}}/Info.plist",
+  ]
+}
+
+bundle_data("clangxpc_bundle_xpc_service_executable") {
+  public_deps = [
+"//clang-tools-extra/clangd/tool:clangd",
+  ]
+  sources = [
+"$root_out_dir/bin/clangd",
+  ]
+  outputs = [
+"{{bundle_executable_dir}}/{{source_file_part}}",
+  ]
+}
+
+create_bundle("clangd.xpc") {
+  # .app directory structure.
+  # Since this target only exists to be copied into ClangdXPC.framework,
+  # put it in $target_gen_dir, not $root_out_dir.
+  bundle_root_dir = "$target_gen_dir/$target_name"
+  bundle_contents_dir = "$bundle_root_dir/Contents"
+  bundle_executable_dir = "$bundle_contents_dir/MacOS"
+
+  deps = [
+":clangxpc_bundle_xpc_service_executable",
+":clangxpc_bundle_xpc_service_info_plist",
+  ]
+}
+
+##
+# ClangdXPC.framework
+
+write_cmake_config("Info.plist") {
+  input = "../cmake/Info.plist.in"
+  output = "$target_gen_dir/Info.plis

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D55463#1451166 , @tschuett wrote:

> Are you planning to do this recursively?
>  The minimizer does not help much for the following example, while Sema.h 
> contains 10,000 lines of useless code.
>
>   #include "clang/Sema/Sema.h"
>  
>   int foo() {}
>


Not recursively, but on each file *independently* by intercepting `stat` and 
`open` in a custom VFS layer.  The old patch at https://reviews.llvm.org/D53354 
(which Alex points to above) might give you more context.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55463



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

[PATCH] D60139: Add clang-tidy/checks/misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Finds placement-new calls where the size of the pointee type of the placement 
parameter is smaller than the size of the constructed type and the pointer is 
implicitly cast to ``void *``


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int * ptr = new int;
+  new (ptr) Dummy;
+  delete ptr;
+
+}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-placement-new-target-size]
+
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,9 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,36 @@
+//===--- PlacementNewTargetSizeCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into allocated
+/// space where the pointer to that allocated space is of a type smaller than the
+/// constructed object and the pointer is implicitly cast to void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.cpp - clang-tidy -===//
+//
+// 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 
+
+#include "PlacementNewTargetSizeCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatch

[PATCH] D60108: [os_log] Mark os_log_helper `nounwind`

[Jonas Devlieghere via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D60108



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

Default -fomit-frame-pointer for PowerPC

[George Koehler via cfe-commits]

Hello cfe-commits,

The attached patch is for clang to use -fomit-frame-pointer by default
for all PowerPC targets when optimizing code.  Right now, clang uses
-fomit-frame-pointer for PowerPC Linux and NetBSD but not for other
targets.  I have been running `clang -target powerpc-openbsd`.

The patch is for llvm-project.git master.  I previously posted this
patch to https://bugs.llvm.org/show_bug.cgi?id=41094 , but the patch
in this email is for a newer revision of master.

In most functions, the frame pointer in r31 is an unnecessary extra
copy of the stack pointer in r1.  GCC is using -fomit-frame-pointer by
default (in my PowerPC machine running OpenBSD/macppc); I want Clang
to be at least as good as GCC.  Also, this patch helps me to compare
the output of `clang -target powerpc-openbsd -O2 -S` with the output
for Linux or NetBSD.  In bug 41094, I showed how -fomit-frame-pointer
simplifies the C function `void nothing(void) {}`.

This is my first mail to cfe-commits; I'm using the instructions at
http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch

-- 
George Koehler 
From a7906214c4751b4c181546ae48ecd05c8f84f07b Mon Sep 17 00:00:00 2001
From: George Koehler 
Date: Tue, 2 Apr 2019 11:29:19 -0400
Subject: [PATCH] Use -fomit-frame-pointer when optimizing PowerPC code

This enables -fomit-frame-pointer when optimizing code for all PowerPC
targets, instead of only Linux and NetBSD.
---
 clang/lib/Driver/ToolChains/Clang.cpp | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a5b98aad54d..6bfca0a5f15 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -523,8 +523,12 @@ static bool useFramePointerForTargetByDefault(const 
ArgList &Args,
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
 return false;
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
+// PowerPC's frame pointer is often an extra copy of the stack pointer.
 return !areOptimizationsEnabled(Args);
   default:
 break;
@@ -542,9 +546,6 @@ static bool useFramePointerForTargetByDefault(const ArgList 
&Args,
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
 case llvm::Triple::mipsel:
-case llvm::Triple::ppc:
-case llvm::Triple::ppc64:
-case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::x86:
 case llvm::Triple::x86_64:
-- 
2.19.1

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

Re: Default -fomit-frame-pointer for PowerPC

[Roman Lebedev via cfe-commits]

Best to submit patches to phabricator https://reviews.llvm.org/

On Tue, Apr 2, 2019 at 8:30 PM George Koehler via cfe-commits
 wrote:
>
> Hello cfe-commits,
>
> The attached patch is for clang to use -fomit-frame-pointer by default
> for all PowerPC targets when optimizing code.  Right now, clang uses
> -fomit-frame-pointer for PowerPC Linux and NetBSD but not for other
> targets.  I have been running `clang -target powerpc-openbsd`.
>
> The patch is for llvm-project.git master.  I previously posted this
> patch to https://bugs.llvm.org/show_bug.cgi?id=41094 , but the patch
> in this email is for a newer revision of master.
>
> In most functions, the frame pointer in r31 is an unnecessary extra
> copy of the stack pointer in r1.  GCC is using -fomit-frame-pointer by
> default (in my PowerPC machine running OpenBSD/macppc); I want Clang
> to be at least as good as GCC.  Also, this patch helps me to compare
> the output of `clang -target powerpc-openbsd -O2 -S` with the output
> for Linux or NetBSD.  In bug 41094, I showed how -fomit-frame-pointer
> simplifies the C function `void nothing(void) {}`.
>
> This is my first mail to cfe-commits; I'm using the instructions at
> http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch
>
> --
> George Koehler 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357501 - [os_log] Mark os_log_helper `nounwind`

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Apr  2 10:42:38 2019
New Revision: 357501

URL: http://llvm.org/viewvc/llvm-project?rev=357501&view=rev
Log:
[os_log] Mark os_log_helper `nounwind`

Allow the optimizer to remove unnecessary EH cleanups surrounding calls
to os_log_helper, to save some code size.

As a follow-up, it might be worthwhile to add a BasicNoexcept exception
spec to os_log_helper, and to then teach CGCall to emit direct calls for
callees which can't throw. This could save some compile-time.

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

Added:
cfe/trunk/test/CodeGenObjCXX/os_log.mm
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=357501&r1=357500&r2=357501&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Apr  2 10:42:38 2019
@@ -1138,6 +1138,7 @@ llvm::Function *CodeGenFunction::generat
   Fn->setVisibility(llvm::GlobalValue::HiddenVisibility);
   CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, Fn);
   CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Fn);
+  Fn->setDoesNotThrow();
 
   // Attach 'noinline' at -Oz.
   if (CGM.getCodeGenOpts().OptimizeSize == 2)

Added: cfe/trunk/test/CodeGenObjCXX/os_log.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/os_log.mm?rev=357501&view=auto
==
--- cfe/trunk/test/CodeGenObjCXX/os_log.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/os_log.mm Tue Apr  2 10:42:38 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple x86_64-darwin-apple -fobjc-arc \
+// RUN:   -fexceptions -fcxx-exceptions -O1 | FileCheck %s
+
+// Check that no EH cleanup is emitted around the call to __os_log_helper.
+namespace no_eh_cleanup {
+  void release(int *lock);
+
+  // CHECK-LABEL: define {{.*}} @_ZN13no_eh_cleanup3logERiPcS1_(
+  void log(int &i, char *data, char *buf) {
+  int lock __attribute__((cleanup(release)));
+  // CHECK: call void @__os_log_helper_1_2_2_4_0_8_34(
+  // CHECK-NEXT: call void @_ZN13no_eh_cleanup7releaseEPi
+  __builtin_os_log_format(buf, "%d %{public}s", i, data);
+  }
+
+  // CHECK: define {{.*}} @__os_log_helper_1_2_2_4_0_8_34({{.*}} 
[[NUW:#[0-9]+]]
+}
+
+// CHECK: attributes [[NUW]] = { {{.*}}nounwind


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

[PATCH] D60108: [os_log] Mark os_log_helper `nounwind`

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357501: [os_log] Mark os_log_helper `nounwind` (authored by 
vedantk, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D60108?vs=193217&id=193322#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60108

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenObjCXX/os_log.mm


Index: test/CodeGenObjCXX/os_log.mm
===
--- test/CodeGenObjCXX/os_log.mm
+++ test/CodeGenObjCXX/os_log.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple x86_64-darwin-apple -fobjc-arc \
+// RUN:   -fexceptions -fcxx-exceptions -O1 | FileCheck %s
+
+// Check that no EH cleanup is emitted around the call to __os_log_helper.
+namespace no_eh_cleanup {
+  void release(int *lock);
+
+  // CHECK-LABEL: define {{.*}} @_ZN13no_eh_cleanup3logERiPcS1_(
+  void log(int &i, char *data, char *buf) {
+  int lock __attribute__((cleanup(release)));
+  // CHECK: call void @__os_log_helper_1_2_2_4_0_8_34(
+  // CHECK-NEXT: call void @_ZN13no_eh_cleanup7releaseEPi
+  __builtin_os_log_format(buf, "%d %{public}s", i, data);
+  }
+
+  // CHECK: define {{.*}} @__os_log_helper_1_2_2_4_0_8_34({{.*}} 
[[NUW:#[0-9]+]]
+}
+
+// CHECK: attributes [[NUW]] = { {{.*}}nounwind
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1138,6 +1138,7 @@
   Fn->setVisibility(llvm::GlobalValue::HiddenVisibility);
   CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, Fn);
   CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Fn);
+  Fn->setDoesNotThrow();
 
   // Attach 'noinline' at -Oz.
   if (CGM.getCodeGenOpts().OptimizeSize == 2)


Index: test/CodeGenObjCXX/os_log.mm
===
--- test/CodeGenObjCXX/os_log.mm
+++ test/CodeGenObjCXX/os_log.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple x86_64-darwin-apple -fobjc-arc \
+// RUN:   -fexceptions -fcxx-exceptions -O1 | FileCheck %s
+
+// Check that no EH cleanup is emitted around the call to __os_log_helper.
+namespace no_eh_cleanup {
+  void release(int *lock);
+
+  // CHECK-LABEL: define {{.*}} @_ZN13no_eh_cleanup3logERiPcS1_(
+  void log(int &i, char *data, char *buf) {
+  int lock __attribute__((cleanup(release)));
+  // CHECK: call void @__os_log_helper_1_2_2_4_0_8_34(
+  // CHECK-NEXT: call void @_ZN13no_eh_cleanup7releaseEPi
+  __builtin_os_log_format(buf, "%d %{public}s", i, data);
+  }
+
+  // CHECK: define {{.*}} @__os_log_helper_1_2_2_4_0_8_34({{.*}} [[NUW:#[0-9]+]]
+}
+
+// CHECK: attributes [[NUW]] = { {{.*}}nounwind
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1138,6 +1138,7 @@
   Fn->setVisibility(llvm::GlobalValue::HiddenVisibility);
   CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, Fn);
   CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Fn);
+  Fn->setDoesNotThrow();
 
   // Attach 'noinline' at -Oz.
   if (CGM.getCodeGenOpts().OptimizeSize == 2)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Default -fomit-frame-pointer for PowerPC

[Finkel, Hal J. via cfe-commits]

On 4/2/19 12:31 PM, Roman Lebedev via cfe-commits wrote:
> Best to submit patches to phabricator https://reviews.llvm.org/


Please follow the instructions here:
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
- This will make it much easier for us to track this.

Thanks!

 -Hal

>
> On Tue, Apr 2, 2019 at 8:30 PM George Koehler via cfe-commits
>  wrote:
>> Hello cfe-commits,
>>
>> The attached patch is for clang to use -fomit-frame-pointer by default
>> for all PowerPC targets when optimizing code.  Right now, clang uses
>> -fomit-frame-pointer for PowerPC Linux and NetBSD but not for other
>> targets.  I have been running `clang -target powerpc-openbsd`.
>>
>> The patch is for llvm-project.git master.  I previously posted this
>> patch to https://bugs.llvm.org/show_bug.cgi?id=41094 , but the patch
>> in this email is for a newer revision of master.
>>
>> In most functions, the frame pointer in r31 is an unnecessary extra
>> copy of the stack pointer in r1.  GCC is using -fomit-frame-pointer by
>> default (in my PowerPC machine running OpenBSD/macppc); I want Clang
>> to be at least as good as GCC.  Also, this patch helps me to compare
>> the output of `clang -target powerpc-openbsd -O2 -S` with the output
>> for Linux or NetBSD.  In bug 41094, I showed how -fomit-frame-pointer
>> simplifies the C function `void nothing(void) {}`.
>>
>> This is my first mail to cfe-commits; I'm using the instructions at
>> http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch
>>
>> --
>> George Koehler 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 created this revision.
ashi1 added a reviewer: yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Do not produce Fat binary functions for HIP when no device code is present.


Repository:
  rC Clang

https://reviews.llvm.org/D60141

Files:
  lib/CodeGen/CGCUDANV.cpp
  test/CodeGenCUDA/device-stub.cu


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -228,14 +228,21 @@
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
 // CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
-// HIPNOGLOBALS: @{{.*}} = internal constant{{.*}}
+// HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// NOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// CUDANOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
-// NOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+// CUDANOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
+// CUDANOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+
+// There should be no fat binary functions when no device-code is found for 
HIP.
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// HIPNOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
+// HIPNOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+

 // There should be no constructors/destructors if we have no GPU binary.
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_ctor
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -472,13 +472,15 @@
   StringRef CudaGpuBinaryFileName = CGM.getCodeGenOpts().CudaGpuBinaryFileName;
   if (CudaGpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
+  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+return nullptr;

   // void __{cuda|hip}_register_globals(void* handle);
   llvm::Function *RegisterGlobalsFunc = makeRegisterGlobalsFn();
   // We always need a function to pass in as callback. Create a dummy
   // implementation if we don't need to register anything.
   if (RelocatableDeviceCode && !RegisterGlobalsFunc)
 RegisterGlobalsFunc = makeDummyFunction(getRegisterGlobalsFnTy());

   // void ** __{cuda|hip}RegisterFatBinary(void *);
   llvm::FunctionCallee RegisterFatbinFunc = CGM.CreateRuntimeFunction(


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -228,14 +228,21 @@
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
 // CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
-// HIPNOGLOBALS: @{{.*}} = internal constant{{.*}}
+// HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// NOGLOBALS: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// CUDANOGLOBALS: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
-// NOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+// CUDANOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
+// CUDANOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+
+// There should be no fat binary functions when no device-code is found for HIP.
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// HIPNOGLOBALS-NOT: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
+// HIPNOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+

 // There should be no constructors/destructors if we have no GPU binary.
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_module_ctor
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -472,13 +472,15 @@
   StringRef CudaGpuBinaryFileName =

r357505 - [driver] clang-format. Fix indentation, split long lines. NFC

[Simon Atanasyan via cfe-commits]

Author: atanasyan
Date: Tue Apr  2 11:03:15 2019
New Revision: 357505

URL: http://llvm.org/viewvc/llvm-project?rev=357505&view=rev
Log:
[driver] clang-format. Fix indentation, split long lines. NFC

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=357505&r1=357504&r2=357505&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Apr  2 11:03:15 2019
@@ -246,8 +246,9 @@ InputArgList Driver::ParseArgStrings(Arr
   : diag::err_drv_unknown_argument;
   Diags.Report(DiagID) << ArgString;
 } else {
-  DiagID = IsCLMode() ? 
diag::warn_drv_unknown_argument_clang_cl_with_suggestion
-  : diag::err_drv_unknown_argument_with_suggestion;
+  DiagID = IsCLMode()
+   ? diag::warn_drv_unknown_argument_clang_cl_with_suggestion
+   : diag::err_drv_unknown_argument_with_suggestion;
   Diags.Report(DiagID) << ArgString << Nearest;
 }
 ContainsError |= Diags.getDiagnosticLevel(DiagID, SourceLocation()) >
@@ -1417,11 +1418,13 @@ void Driver::generateCompilationDiagnost
 }
 
 void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) {
-  // Since commandLineFitsWithinSystemLimits() may underestimate system's 
capacity
-  // if the tool does not support response files, there is a chance/ that 
things
-  // will just work without a response file, so we silently just skip it.
+  // Since commandLineFitsWithinSystemLimits() may underestimate system's
+  // capacity if the tool does not support response files, there is a chance/
+  // that things will just work without a response file, so we silently just
+  // skip it.
   if (Cmd.getCreator().getResponseFilesSupport() == Tool::RF_None ||
-  llvm::sys::commandLineFitsWithinSystemLimits(Cmd.getExecutable(), 
Cmd.getArguments()))
+  llvm::sys::commandLineFitsWithinSystemLimits(Cmd.getExecutable(),
+   Cmd.getArguments()))
 return;
 
   std::string TmpName = GetTemporaryPath("response", "txt");
@@ -2030,7 +2033,8 @@ void Driver::BuildInputs(const ToolChain
 
 Arg *Previous = nullptr;
 bool ShowNote = false;
-for (Arg *A : Args.filtered(options::OPT__SLASH_TC, 
options::OPT__SLASH_TP)) {
+for (Arg *A :
+ Args.filtered(options::OPT__SLASH_TC, options::OPT__SLASH_TP)) {
   if (Previous) {
 Diag(clang::diag::warn_drv_overriding_flag_option)
   << Previous->getSpelling() << A->getSpelling();
@@ -4260,8 +4264,8 @@ const char *Driver::GetNamedOutputPath(C
   SmallString<128> CrashDirectory(A->getValue());
   llvm::sys::path::append(CrashDirectory, Split.first);
   const char *Middle = Suffix ? "-%%." : "-%%";
-  std::error_code EC =
-  llvm::sys::fs::createUniqueFile(CrashDirectory + Middle + Suffix, 
TmpName);
+  std::error_code EC = llvm::sys::fs::createUniqueFile(
+  CrashDirectory + Middle + Suffix, TmpName);
   if (EC) {
 Diag(clang::diag::err_unable_to_make_temp) << EC.message();
 return "";
@@ -4764,7 +4768,8 @@ bool Driver::GetReleaseVersion(StringRef
   return false;
 }
 
-std::pair Driver::getIncludeExcludeOptionFlagMasks(bool 
IsClCompatMode) const {
+std::pair
+Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
   unsigned IncludedFlagsBitmask = 0;
   unsigned ExcludedFlagsBitmask = options::NoDriverOption;
 

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=357505&r1=357504&r2=357505&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Apr  2 11:03:15 2019
@@ -464,7 +464,7 @@ void tools::gnutools::Linker::ConstructJ
 
   if (HasCRTBeginEndFiles)
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
-   }
+}
 
 // Add crtfastmath.o if available and fast math is enabled.
 ToolChain.AddFastMathRuntimeIfAvailable(Args, CmdArgs);
@@ -675,14 +675,16 @@ void tools::gnutools::Assembler::Constru
   case llvm::Triple::sparcel: {
 CmdArgs.push_back("-32");
 std::string CPU = getCPUName(Args, getToolChain().getTriple());
-CmdArgs.push_back(sparc::getSparcAsmModeForCPU(CPU, 
getToolChain().getTriple()));
+CmdArgs.push_back(
+sparc::getSparcAsmModeForCPU(CPU, getToolChain().getTriple()));
 AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
 break;
   }
   case llvm::Triple::sparcv9: {
 CmdArgs.push_back("-64");
 std::string CPU = getCPUName(Args, getToolChain().getTriple())

r357506 - [driver][mips] Check both `gnuabi64` and `gnu` suffixes in `getMultiarchTriple`

[Simon Atanasyan via cfe-commits]

Author: atanasyan
Date: Tue Apr  2 11:03:31 2019
New Revision: 357506

URL: http://llvm.org/viewvc/llvm-project?rev=357506&view=rev
Log:
[driver][mips] Check both `gnuabi64` and `gnu` suffixes in `getMultiarchTriple`

In case of N64 ABI toolchain paths migth have `mips-linux-gnuabi64`
or `mips-linux-gnu` directory regardless of selected environment.
Check both variants while detecting a multiarch triple.

Fix for the bug https://bugs.llvm.org/show_bug.cgi?id=41204

Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=357506&r1=357505&r2=357506&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Apr  2 11:03:31 2019
@@ -44,6 +44,7 @@ static std::string getMultiarchTriple(co
   TargetTriple.getEnvironment();
   bool IsAndroid = TargetTriple.isAndroid();
   bool IsMipsR6 = TargetTriple.getSubArch() == llvm::Triple::MipsSubArch_r6;
+  bool IsMipsN32Abi = TargetTriple.getEnvironment() == llvm::Triple::GNUABIN32;
 
   // For most architectures, just use whatever we have rather than trying to be
   // clever.
@@ -102,33 +103,37 @@ static std::string getMultiarchTriple(co
   return "aarch64_be-linux-gnu";
 break;
   case llvm::Triple::mips: {
-std::string Arch = IsMipsR6 ? "mipsisa32r6" : "mips";
-if (D.getVFS().exists(SysRoot + "/lib/" + Arch + "-linux-gnu"))
-  return Arch + "-linux-gnu";
+std::string MT = IsMipsR6 ? "mipsisa32r6-linux-gnu" : "mips-linux-gnu";
+if (D.getVFS().exists(SysRoot + "/lib/" + MT))
+  return MT;
 break;
   }
   case llvm::Triple::mipsel: {
 if (IsAndroid)
   return "mipsel-linux-android";
-std::string Arch = IsMipsR6 ? "mipsisa32r6el" : "mipsel";
-if (D.getVFS().exists(SysRoot + "/lib/" + Arch + "-linux-gnu"))
-  return Arch + "-linux-gnu";
+std::string MT = IsMipsR6 ? "mipsisa32r6el-linux-gnu" : "mipsel-linux-gnu";
+if (D.getVFS().exists(SysRoot + "/lib/" + MT))
+  return MT;
 break;
   }
   case llvm::Triple::mips64: {
-std::string Arch = IsMipsR6 ? "mipsisa64r6" : "mips64";
-std::string ABI = llvm::Triple::getEnvironmentTypeName(TargetEnvironment);
-if (D.getVFS().exists(SysRoot + "/lib/" + Arch + "-linux-" + ABI))
-  return Arch + "-linux-" + ABI;
+std::string MT = std::string(IsMipsR6 ? "mipsisa64r6" : "mips64") +
+ "-linux-" + (IsMipsN32Abi ? "gnuabin32" : "gnuabi64");
+if (D.getVFS().exists(SysRoot + "/lib/" + MT))
+  return MT;
+if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnu"))
+  return "mips64-linux-gnu";
 break;
   }
   case llvm::Triple::mips64el: {
 if (IsAndroid)
   return "mips64el-linux-android";
-std::string Arch = IsMipsR6 ? "mipsisa64r6el" : "mips64el";
-std::string ABI = llvm::Triple::getEnvironmentTypeName(TargetEnvironment);
-if (D.getVFS().exists(SysRoot + "/lib/" + Arch + "-linux-" + ABI))
-  return Arch + "-linux-" + ABI;
+std::string MT = std::string(IsMipsR6 ? "mipsisa64r6el" : "mips64el") +
+ "-linux-" + (IsMipsN32Abi ? "gnuabin32" : "gnuabi64");
+if (D.getVFS().exists(SysRoot + "/lib/" + MT))
+  return MT;
+if (D.getVFS().exists(SysRoot + "/lib/mips64el-linux-gnu"))
+  return "mips64el-linux-gnu";
 break;
   }
   case llvm::Triple::ppc:

Modified: cfe/trunk/test/Driver/linux-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-ld.c?rev=357506&r1=357505&r2=357506&view=diff
==
--- cfe/trunk/test/Driver/linux-ld.c (original)
+++ cfe/trunk/test/Driver/linux-ld.c Tue Apr  2 11:03:31 2019
@@ -1660,6 +1660,11 @@
 // CHECK-DEBIAN-ML-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=mips64-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/debian_6_mips64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-DEBIAN-ML-MIPS64-GNUABI %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=mips64-linux-gnuabi64 -mabi=n64 \
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/debian_6_mips64_tree \
@@ -1680,6 +1685,11 @@
 // CHECK-DEBIAN-ML-MIPS64-GNUABI: 
"{{.*}}/usr/lib/gcc/mips64-linux-gnuabi64/4.9/../../../mips64-linux-gnuabi64{{/|}}crtn.o"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=mips64el-unknown-linux-gnu \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/debian_6_mips64_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-DEBIAN-ML-MIPS64EL-GNUABI %s
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:10
+#include 
+
+#include "PlacementNewTargetSizeCheck.h"

Unnecessary empty line. Please run Clang-format after fixing.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:23
+namespace {
+CharUnits getSizeOfType(clang::ASTContext *const Context, const QualType& 
type) {
+  CharUnits TypeSize;

Please use static instead of anonymous namespaces. See LLVM Coding Guidelines.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:26
+  if (type->isRecordType()) {
+const auto *CXXRecordDecl = type->getAsRecordDecl();
+assert(CXXRecordDecl && "type->getAsRecordDecl() failed");

Please don't use auto where type could not be easily deduced.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:28
+assert(CXXRecordDecl && "type->getAsRecordDecl() failed");
+const auto &CXXDeclLayout =
+CXXRecordDecl->getASTContext().getASTRecordLayout(CXXRecordDecl);

Please don't use auto where type could not be easily deduced.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:47
+const MatchFinder::MatchResult &Result) {
+  const CXXNewExpr *Alloc = Result.Nodes.getNodeAs("Alloc");
+  assert(Alloc && "Matched node bound by 'Alloc' shoud be a 'CXXNewExpr'");

auto could be used here, because of cast.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:50
+
+  auto numPlacementArgs = Alloc->getNumPlacementArgs();
+  if (0 == numPlacementArgs) {

Please don't use auto where type could not be easily deduced.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:55
+
+  auto type = Alloc->getAllocatedType();
+  CharUnits TypeSize = getSizeOfType(Result.Context, type);

Please don't use auto where type could not be easily deduced.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:62
+  // get ptr type of implicit cast
+  const ImplicitCastExpr *Cast = 
Result.Nodes.getNodeAs("Cast");
+  auto CastType = Cast->getSubExpr()->getType();

auto could be used here, because of cast.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:63
+  const ImplicitCastExpr *Cast = 
Result.Nodes.getNodeAs("Cast");
+  auto CastType = Cast->getSubExpr()->getType();
+  if (CastType->isPointerType()) {

Please don't use auto where type could not be easily deduced.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:65
+  if (CastType->isPointerType()) {
+   CharUnits CastSize = getSizeOfType(Result.Context, 
CastType->getPointeeType());
+if ((TypeSize - CastSize).isPositive()) {

Please run Clang-format.



Comment at: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp:73
+  }
+
+}

Unnecessary empty line.



Comment at: docs/ReleaseNotes.rst:135
+  ` check.
+
 - New :doc:`openmp-exception-escape

Please add one sentence description. Should be same as in documentation.



Comment at: docs/clang-tidy/checks/misc-placement-new-target-size.rst:5
+==
+
+

Unnecessary empty line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60139



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

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think I've addressed all the comments, and I know that @ruiu is currently 
travelling or recovering from travel to Japan, so I'll go ahead and land this. 
Feel free to provide post-commit feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59797



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

[PATCH] D56924: Special case ObjCPropertyDecl for printing

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.
Herald added a subscriber: dexonsmith.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D56924



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 193340.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Add a test case where a macro emits just `self` in the message expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+#define INITIALIZE() [super self]
+
+- (instancetype)initWithObject:(NSObject *)objc a:(int)a {
+  return INITIALIZE();
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+#define INITIALIZER_IMPL() return [super self]
+
+- (instancetype)initWithObject:(NSObject *)objc b:(int)b {
+  INITIALIZER_IMPL();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+}
+
+#define INITIALIZER_METHOD self
+
+- (instancetype)initWithObject:(NSObject *)objc c:(int)c {
+  return [super INITIALIZER_METHOD];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of ``-self`` on super instances in initializers of subclasses
+of ``NSObject`` and recommends calling a superclass initializer instead.
+
+Invoking ``-self`` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,12 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of ``-self`` on super instances in initializers of
+  subclasses of ``NSObject`` and recommends calling a superclass initializer
+  instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/cla

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),
+  StringRef("[super init]"));

aaron.ballman wrote:
> stephanemoore wrote:
> > stephanemoore wrote:
> > > aaron.ballman wrote:
> > > > This could be dangerous if the `[super self]` construct is in a macro, 
> > > > couldn't it? e.g.,
> > > > ```
> > > > #define DERP self
> > > > 
> > > > [super DERP];
> > > > ```
> > > Good point. Let me add some test cases and make sure this is handled 
> > > properly.
> > Added some test cases where `[super self]` is expanded from macros.
> You missed the test case I was worried about -- where the macro is mixed into 
> the expression. I don't think we want to try to add a fix-it in that case.
Added a test case though at the moment it generates a fixit.

Before I investigate modifying the check to avoid generating a fixit in this 
case, I think it would be helpful for me to understand your concerns in that 
scenario better. Is your concern that a proper fix might involve fixing the 
macro itself rather than the message expression?

```
#if FLAG
#define INVOCATION self
#else
#define INVOCATION init
#endif

- (instancetype)init {
  return [super INVOCATION];
}
```



Comment at: clang-tools-extra/test/clang-tidy/objc-super-self.m:41
+  INITIALIZER_IMPL();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in 
initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+}

aaron.ballman wrote:
> Are you missing a `CHECK-FIXES` here?
> 
> Personally, I don't think we should try to generate a fixit for this case, so 
> I would expect a CHECK-FIXES that ensures this doesn't get modified.
No fix is currently generated for this case which is why there is no 
`CHECK-FIXES`. I also agree that no fix should be generated for this case.

I must confess that I have yet to fully understand why the fix for this case is 
discarded (though I am grateful for the behavior). Let me dig around to try to 
better understand why no fixit is generated for this case and assess adding a 
condition for emitting the fixit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

r357514 - [OPENMP]Add codegen for private vars with allocate clause.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr  2 12:44:46 2019
New Revision: 357514

URL: http://llvm.org/viewvc/llvm-project?rev=357514&view=rev
Log:
[OPENMP]Add codegen for private vars with allocate clause.

Added codegen/test for the privatized variables with the allocate
clause.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/parallel_private_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=357514&r1=357513&r2=357514&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Apr  2 12:44:46 2019
@@ -9761,13 +9761,9 @@ Address CGOpenMPRuntime::getAddressOfLoc
 return Address::invalid();
   const auto *AA = CVD->getAttr();
   // Use the default allocation.
-  if (AA->getAllocatorType() == OMPAllocateDeclAttr::OMPDefaultMemAlloc)
+  if (AA->getAllocatorType() == OMPAllocateDeclAttr::OMPDefaultMemAlloc &&
+  !AA->getAllocator())
 return Address::invalid();
-  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
-  if (!Elem.second.ServiceInsertPt)
-setLocThreadIdInsertPt(CGF);
-  CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
-  CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
   llvm::Value *Size;
   CharUnits Align = CGM.getContext().getDeclAlign(CVD);
   if (CVD->getType()->isVariablyModifiedType()) {

Modified: cfe/trunk/test/OpenMP/parallel_private_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_private_codegen.cpp?rev=357514&r1=357513&r2=357514&view=diff
==
--- cfe/trunk/test/OpenMP/parallel_private_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_private_codegen.cpp Tue Apr  2 12:44:46 2019
@@ -13,6 +13,16 @@
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
+typedef void **omp_allocator_handle_t;
+extern const omp_allocator_handle_t omp_default_mem_alloc;
+extern const omp_allocator_handle_t omp_large_cap_mem_alloc;
+extern const omp_allocator_handle_t omp_const_mem_alloc;
+extern const omp_allocator_handle_t omp_high_bw_mem_alloc;
+extern const omp_allocator_handle_t omp_low_lat_mem_alloc;
+extern const omp_allocator_handle_t omp_cgroup_mem_alloc;
+extern const omp_allocator_handle_t omp_pteam_mem_alloc;
+extern const omp_allocator_handle_t omp_thread_mem_alloc;
+
 template 
 struct S {
   T f;
@@ -54,7 +64,7 @@ template
 struct SST {
   T a;
   SST() : a(T()) {
-#pragma omp parallel private(a)
+#pragma omp parallel private(a) allocate(omp_large_cap_mem_alloc:a)
 #ifdef LAMBDA
 [&]() {
   [&]() {
@@ -343,12 +353,18 @@ int main() {
 // CHECK: ret
 
 // CHECK: define internal void [[SST_MICROTASK]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, [[SST_TY]]* %{{.+}})
-// CHECK: [[A_PRIV:%.+]] = alloca i{{[0-9]+}},
+// CHECK: [[GTID_ADDR_PTR:%.+]] = alloca i32*,
+// CHECK: [[GTID_ADDR:%.+]] = load i32*, i32** [[GTID_ADDR_PTR]],
+// CHECK: [[GTID:%.+]] = load i32, i32* [[GTID_ADDR]],
+// CHECK: [[LARGE_CAP_ALLOC:%.+]] = load i8**, i8*** @omp_large_cap_mem_alloc,
+// CHECK: [[A_VOID_PTR:%.+]] = call i8* @__kmpc_alloc(i32 [[GTID]], i64 4, 
i8** [[LARGE_CAP_ALLOC]])
+// CHECK: [[A_PRIV:%.+]] = bitcast i8* [[A_VOID_PTR]] to i32*
 // CHECK: store i{{[0-9]+}}* [[A_PRIV]], i{{[0-9]+}}** [[REF:%.+]],
 // CHECK-NEXT: [[A_PRIV:%.+]] = load i{{[0-9]+}}*, i{{[0-9]+}}** [[REF]],
 // CHECK-NEXT: [[A_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[A_PRIV]],
 // CHECK-NEXT: [[INC:%.+]] = add nsw i{{[0-9]+}} [[A_VAL]], 1
 // CHECK-NEXT: store i{{[0-9]+}} [[INC]], i{{[0-9]+}}* [[A_PRIV]],
+// CHECK-NEXT: call void @__kmpc_free(i32 [[GTID]], i8* [[A_VOID_PTR]], i8** 
[[LARGE_CAP_ALLOC]])
 // CHECK-NEXT: ret void
 
 #endif


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

r357516 - [Sema] Fix a use-after-deallocate of a ParsedAttr

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Apr  2 12:48:11 2019
New Revision: 357516

URL: http://llvm.org/viewvc/llvm-project?rev=357516&view=rev
Log:
[Sema] Fix a use-after-deallocate of a ParsedAttr

moveAttrFromListToList only makes sense when moving an attribute to a list with
a pool that's either equivalent, or has a shorter lifetime. Therefore, using it
to move a ParsedAttr from a declarator to a declaration specifier doesn't make
sense, since the declaration specifier's pool outlives the declarator's. The
patch adds a new function, ParsedAttributes::takeOneFrom, which transfers the
attribute from one pool to another, fixing the use-after-deallocate.

rdar://49175426

Differential revision: https://reviews.llvm.org/D60101

Modified:
cfe/trunk/include/clang/Sema/ParsedAttr.h
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m

Modified: cfe/trunk/include/clang/Sema/ParsedAttr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ParsedAttr.h?rev=357516&r1=357515&r2=357516&view=diff
==
--- cfe/trunk/include/clang/Sema/ParsedAttr.h (original)
+++ cfe/trunk/include/clang/Sema/ParsedAttr.h Tue Apr  2 12:48:11 2019
@@ -659,6 +659,7 @@ public:
 
 class AttributePool {
   friend class AttributeFactory;
+  friend class ParsedAttributes;
   AttributeFactory &Factory;
   llvm::TinyPtrVector Attrs;
 
@@ -892,6 +893,13 @@ public:
 pool.takeAllFrom(attrs.pool);
   }
 
+  void takeOneFrom(ParsedAttributes &Attrs, ParsedAttr *PA) {
+Attrs.getPool().remove(PA);
+Attrs.remove(PA);
+getPool().add(PA);
+addAtEnd(PA);
+  }
+
   void clear() {
 clearListOnly();
 pool.clear();

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=357516&r1=357515&r2=357516&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Apr  2 12:48:11 2019
@@ -534,8 +534,8 @@ static void distributeObjCPointerTypeAtt
   // attribute from being applied multiple times and gives
   // the source-location-filler something to work with.
   state.saveDeclSpecAttrs();
-  moveAttrFromListToList(attr, declarator.getAttributes(),
- declarator.getMutableDeclSpec().getAttributes());
+  declarator.getMutableDeclSpec().getAttributes().takeOneFrom(
+  declarator.getAttributes(), &attr);
   return;
 }
   }

Modified: cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m?rev=357516&r1=357515&r2=357516&view=diff
==
--- cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m (original)
+++ cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m Tue Apr  2 12:48:11 2019
@@ -287,3 +287,7 @@ __attribute__((objc_root_class))
 @synthesize collision = _collision; // expected-note {{property synthesized 
here}}
 
 @end
+
+// This used to crash because we'd temporarly store the weak attribute on the
+// declaration specifier, then deallocate it when clearing the declarator.
+id i1, __weak i2, i3;


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

r357515 - [CodeGen] Fix a regression by emitting lambda expressions in EmitLValue

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Apr  2 12:48:07 2019
New Revision: 357515

URL: http://llvm.org/viewvc/llvm-project?rev=357515&view=rev
Log:
[CodeGen] Fix a regression by emitting lambda expressions in EmitLValue

This ability was removed in r351487, but it's needed when a lambda appears as an
OpaqueValueExpr subexpression of a PseudoObjectExpr.

rdar://49030379

Differential revision: https://reviews.llvm.org/D60099

Added:
cfe/trunk/test/CodeGenObjCXX/property-lvalue-lambda.mm
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=357515&r1=357514&r2=357515&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Apr  2 12:48:07 2019
@@ -1294,6 +1294,8 @@ LValue CodeGenFunction::EmitLValue(const
 return EmitCXXBindTemporaryLValue(cast(E));
   case Expr::CXXUuidofExprClass:
 return EmitCXXUuidofLValue(cast(E));
+  case Expr::LambdaExprClass:
+return EmitAggExprToLValue(E);
 
   case Expr::ExprWithCleanupsClass: {
 const auto *cleanups = cast(E);

Added: cfe/trunk/test/CodeGenObjCXX/property-lvalue-lambda.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/property-lvalue-lambda.mm?rev=357515&view=auto
==
--- cfe/trunk/test/CodeGenObjCXX/property-lvalue-lambda.mm (added)
+++ cfe/trunk/test/CodeGenObjCXX/property-lvalue-lambda.mm Tue Apr  2 12:48:07 
2019
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fblocks -disable-llvm-passes -triple x86_64-apple-darwin10 
-std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+typedef void (^blk_t)();
+typedef void (*fnptr_t)();
+
+@interface X
+@property blk_t blk;
+@property fnptr_t fnptr;
+@end
+
+template 
+blk_t operator+(blk_t lhs, T) { return lhs; }
+
+template 
+fnptr_t operator+(fnptr_t lhs, T) { return lhs; }
+
+// CHECK-LABEL: define void @_Z2t1P1X
+void t1(X *x) {
+  // Check that we call lambda.operator blk_t(), and that we send that result 
to
+  // the setter.
+
+  // CHECK: [[CALL:%.*]] = call void ()* 
@"_ZZ2t1P1XENK3$_0cvU13block_pointerFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL]])
+  x.blk = [] {};
+
+  // CHECK: [[CALL2:%.*]] = call void ()* @"_ZZ2t1P1XENK3$_1cvPFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL2]])
+  x.fnptr = [] {};
+}
+
+// CHECK-LABEL: define void @_Z2t2P1X
+void t2(X *x) {
+  // Test the case when the lambda isn't unique. (see 
OpaqueValueExpr::isUnique)
+  // FIXME: This asserts if the lambda isn't trivially copy/movable.
+
+  // [x setBlk: operator+([x blk], [] {})]
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* 
@"_ZplIZ2t2P1XE3$_2EU13block_pointerFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.blk += [] {};
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* @"_ZplIZ2t2P1XE3$_3EPFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.fnptr += [] {};
+}


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

[PATCH] D60099: [CodeGen] Fix a regression by emitting lambda expressions in EmitLValue

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357515: [CodeGen] Fix a regression by emitting lambda 
expressions in EmitLValue (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60099?vs=193179&id=193347#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60099

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenObjCXX/property-lvalue-lambda.mm


Index: test/CodeGenObjCXX/property-lvalue-lambda.mm
===
--- test/CodeGenObjCXX/property-lvalue-lambda.mm
+++ test/CodeGenObjCXX/property-lvalue-lambda.mm
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fblocks -disable-llvm-passes -triple x86_64-apple-darwin10 
-std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+typedef void (^blk_t)();
+typedef void (*fnptr_t)();
+
+@interface X
+@property blk_t blk;
+@property fnptr_t fnptr;
+@end
+
+template 
+blk_t operator+(blk_t lhs, T) { return lhs; }
+
+template 
+fnptr_t operator+(fnptr_t lhs, T) { return lhs; }
+
+// CHECK-LABEL: define void @_Z2t1P1X
+void t1(X *x) {
+  // Check that we call lambda.operator blk_t(), and that we send that result 
to
+  // the setter.
+
+  // CHECK: [[CALL:%.*]] = call void ()* 
@"_ZZ2t1P1XENK3$_0cvU13block_pointerFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL]])
+  x.blk = [] {};
+
+  // CHECK: [[CALL2:%.*]] = call void ()* @"_ZZ2t1P1XENK3$_1cvPFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL2]])
+  x.fnptr = [] {};
+}
+
+// CHECK-LABEL: define void @_Z2t2P1X
+void t2(X *x) {
+  // Test the case when the lambda isn't unique. (see 
OpaqueValueExpr::isUnique)
+  // FIXME: This asserts if the lambda isn't trivially copy/movable.
+
+  // [x setBlk: operator+([x blk], [] {})]
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* 
@"_ZplIZ2t2P1XE3$_2EU13block_pointerFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.blk += [] {};
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* @"_ZplIZ2t2P1XE3$_3EPFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.fnptr += [] {};
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1294,6 +1294,8 @@
 return EmitCXXBindTemporaryLValue(cast(E));
   case Expr::CXXUuidofExprClass:
 return EmitCXXUuidofLValue(cast(E));
+  case Expr::LambdaExprClass:
+return EmitAggExprToLValue(E);
 
   case Expr::ExprWithCleanupsClass: {
 const auto *cleanups = cast(E);


Index: test/CodeGenObjCXX/property-lvalue-lambda.mm
===
--- test/CodeGenObjCXX/property-lvalue-lambda.mm
+++ test/CodeGenObjCXX/property-lvalue-lambda.mm
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fblocks -disable-llvm-passes -triple x86_64-apple-darwin10 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+typedef void (^blk_t)();
+typedef void (*fnptr_t)();
+
+@interface X
+@property blk_t blk;
+@property fnptr_t fnptr;
+@end
+
+template 
+blk_t operator+(blk_t lhs, T) { return lhs; }
+
+template 
+fnptr_t operator+(fnptr_t lhs, T) { return lhs; }
+
+// CHECK-LABEL: define void @_Z2t1P1X
+void t1(X *x) {
+  // Check that we call lambda.operator blk_t(), and that we send that result to
+  // the setter.
+
+  // CHECK: [[CALL:%.*]] = call void ()* @"_ZZ2t1P1XENK3$_0cvU13block_pointerFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL]])
+  x.blk = [] {};
+
+  // CHECK: [[CALL2:%.*]] = call void ()* @"_ZZ2t1P1XENK3$_1cvPFvvEEv"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} void ()* [[CALL2]])
+  x.fnptr = [] {};
+}
+
+// CHECK-LABEL: define void @_Z2t2P1X
+void t2(X *x) {
+  // Test the case when the lambda isn't unique. (see OpaqueValueExpr::isUnique)
+  // FIXME: This asserts if the lambda isn't trivially copy/movable.
+
+  // [x setBlk: operator+([x blk], [] {})]
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* @"_ZplIZ2t2P1XE3$_2EU13block_pointerFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.blk += [] {};
+
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}
+  // CHECK: [[PLUS:%.*]] = call void ()* @"_ZplIZ2t2P1XE3$_3EPFvvES4_T_"
+  // CHECK: call void{{.*}}@objc_msgSend{{.*}}({{.*}} [[PLUS]])
+  x.fnptr += [] {};
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1294,6 +1294,8 @@
 return EmitCXXBindTemporaryLValue(cast(E));
   case Expr::CXXUuidofExprClass:
 return EmitCXXUuidofLValue(cast(E));
+  case Expr::LambdaExprClass:
+return EmitAggExprToLValue(E);
 
   case Expr::ExprWithCleanupsClass: {
 const auto *c

[PATCH] D60101: [Sema] Fix a use-after-deallocate of a ParsedAttr

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357516: [Sema] Fix a use-after-deallocate of a ParsedAttr 
(authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60101?vs=193185&id=193348#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60101

Files:
  include/clang/Sema/ParsedAttr.h
  lib/Sema/SemaType.cpp
  test/SemaObjC/arc-property-decl-attrs.m


Index: test/SemaObjC/arc-property-decl-attrs.m
===
--- test/SemaObjC/arc-property-decl-attrs.m
+++ test/SemaObjC/arc-property-decl-attrs.m
@@ -287,3 +287,7 @@
 @synthesize collision = _collision; // expected-note {{property synthesized 
here}}
 
 @end
+
+// This used to crash because we'd temporarly store the weak attribute on the
+// declaration specifier, then deallocate it when clearing the declarator.
+id i1, __weak i2, i3;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -534,8 +534,8 @@
   // attribute from being applied multiple times and gives
   // the source-location-filler something to work with.
   state.saveDeclSpecAttrs();
-  moveAttrFromListToList(attr, declarator.getAttributes(),
- declarator.getMutableDeclSpec().getAttributes());
+  declarator.getMutableDeclSpec().getAttributes().takeOneFrom(
+  declarator.getAttributes(), &attr);
   return;
 }
   }
Index: include/clang/Sema/ParsedAttr.h
===
--- include/clang/Sema/ParsedAttr.h
+++ include/clang/Sema/ParsedAttr.h
@@ -659,6 +659,7 @@
 
 class AttributePool {
   friend class AttributeFactory;
+  friend class ParsedAttributes;
   AttributeFactory &Factory;
   llvm::TinyPtrVector Attrs;
 
@@ -892,6 +893,13 @@
 pool.takeAllFrom(attrs.pool);
   }
 
+  void takeOneFrom(ParsedAttributes &Attrs, ParsedAttr *PA) {
+Attrs.getPool().remove(PA);
+Attrs.remove(PA);
+getPool().add(PA);
+addAtEnd(PA);
+  }
+
   void clear() {
 clearListOnly();
 pool.clear();


Index: test/SemaObjC/arc-property-decl-attrs.m
===
--- test/SemaObjC/arc-property-decl-attrs.m
+++ test/SemaObjC/arc-property-decl-attrs.m
@@ -287,3 +287,7 @@
 @synthesize collision = _collision; // expected-note {{property synthesized here}}
 
 @end
+
+// This used to crash because we'd temporarly store the weak attribute on the
+// declaration specifier, then deallocate it when clearing the declarator.
+id i1, __weak i2, i3;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -534,8 +534,8 @@
   // attribute from being applied multiple times and gives
   // the source-location-filler something to work with.
   state.saveDeclSpecAttrs();
-  moveAttrFromListToList(attr, declarator.getAttributes(),
- declarator.getMutableDeclSpec().getAttributes());
+  declarator.getMutableDeclSpec().getAttributes().takeOneFrom(
+  declarator.getAttributes(), &attr);
   return;
 }
   }
Index: include/clang/Sema/ParsedAttr.h
===
--- include/clang/Sema/ParsedAttr.h
+++ include/clang/Sema/ParsedAttr.h
@@ -659,6 +659,7 @@
 
 class AttributePool {
   friend class AttributeFactory;
+  friend class ParsedAttributes;
   AttributeFactory &Factory;
   llvm::TinyPtrVector Attrs;
 
@@ -892,6 +893,13 @@
 pool.takeAllFrom(attrs.pool);
   }
 
+  void takeOneFrom(ParsedAttributes &Attrs, ParsedAttr *PA) {
+Attrs.getPool().remove(PA);
+Attrs.remove(PA);
+getPool().add(PA);
+addAtEnd(PA);
+  }
+
   void clear() {
 clearListOnly();
 pool.clear();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60141



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

r357520 - [HIP-Clang] Fat binary should not be produced for non GPU code

[Aaron Enye Shi via cfe-commits]

Author: aaronenyeshi
Date: Tue Apr  2 13:10:18 2019
New Revision: 357520

URL: http://llvm.org/viewvc/llvm-project?rev=357520&view=rev
Log:
[HIP-Clang] Fat binary should not be produced for non GPU code

Skip producing the fat binary functions for HIP when no device code is present.

Reviewers: yaxunl

Differential Review: https://reviews.llvm.org/D60141


Modified:
cfe/trunk/lib/CodeGen/CGCUDANV.cpp
cfe/trunk/test/CodeGenCUDA/device-stub.cu

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=357520&r1=357519&r2=357520&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Tue Apr  2 13:10:18 2019
@@ -472,6 +472,8 @@ llvm::Function *CGNVCUDARuntime::makeMod
   StringRef CudaGpuBinaryFileName = CGM.getCodeGenOpts().CudaGpuBinaryFileName;
   if (CudaGpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
+  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+return nullptr;
 
   // void __{cuda|hip}_register_globals(void* handle);
   llvm::Function *RegisterGlobalsFunc = makeRegisterGlobalsFn();

Modified: cfe/trunk/test/CodeGenCUDA/device-stub.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/device-stub.cu?rev=357520&r1=357519&r2=357520&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/device-stub.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/device-stub.cu Tue Apr  2 13:10:18 2019
@@ -228,13 +228,19 @@ void hostfunc(void) { kernelfunc<<<1, 1>
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
 // CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
-// HIPNOGLOBALS: @{{.*}} = internal constant{{.*}}
+// HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// NOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// CUDANOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
-// NOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+// CUDANOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
+// CUDANOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
+
+// There should be no fat binary functions when no device-code is found for 
HIP.
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// HIPNOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
+// HIPNOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
 
 // There should be no constructors/destructors if we have no GPU binary.
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals


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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

General nit: please use diffs with very large context when you submit patches 
with  Phabricator.
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface




Comment at: lib/CodeGen/CGCUDANV.cpp:475-476
 return nullptr;
+  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+return nullptr;
   // void __{cuda|hip}_register_globals(void* handle);

I think this would make sense for CUDA, too. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D60141



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

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists accepted this revision.
mclow.lists added a comment.

I'm OK with this.  I think, strictly speaking, that libc++ is following the 
standard, and everyone else is not - but what everyone else is doing makes 
sense.




Comment at: libcxx/include/istream:365
 __input_arithmetic(basic_istream<_CharT, _Traits>& __is, _Tp& __n) {
-#ifndef _LIBCPP_NO_EXCEPTIONS
-try
+ios_base::iostate __state = ios_base::goodbit;
+typename basic_istream<_CharT, _Traits>::sentry __s(__is);

lichray wrote:
> I like `auto` just FYI :)
You may like `auto`, but C++03 does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49863



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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/CGCUDANV.cpp:475-476
 return nullptr;
+  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+return nullptr;
   // void __{cuda|hip}_register_globals(void* handle);

tra wrote:
> I think this would make sense for CUDA, too. 
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGCUDANV.cpp#L482

CUDA generates a dummy register function call for -fgpu-rdc. So probably only 
do this when RelocatableDeviceCode is false for CUDA?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60141



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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.



Comment at: lib/CodeGen/CGCUDANV.cpp:475-476
 return nullptr;
+  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+return nullptr;
   // void __{cuda|hip}_register_globals(void* handle);

yaxunl wrote:
> tra wrote:
> > I think this would make sense for CUDA, too. 
> https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGCUDANV.cpp#L482
> 
> CUDA generates a dummy register function call for -fgpu-rdc. So probably only 
> do this when RelocatableDeviceCode is false for CUDA?
Good point. Off the top of my head I can't tell why -fcudardc needs this. 
Keeping the change HIP-only is fine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60141



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

r357526 - [HIP-Clang] Fat binary should not be produced for non GPU code 2

[Aaron Enye Shi via cfe-commits]

Author: aaronenyeshi
Date: Tue Apr  2 13:49:41 2019
New Revision: 357526

URL: http://llvm.org/viewvc/llvm-project?rev=357526&view=rev
Log:
[HIP-Clang] Fat binary should not be produced for non GPU code 2

Also for CUDA, we need to disable producing these fat binary functions when 
there is no GPU code.

Reviewers: yaxunl, tra

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

Modified:
cfe/trunk/lib/CodeGen/CGCUDANV.cpp
cfe/trunk/test/CodeGenCUDA/device-stub.cu

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=357526&r1=357525&r2=357526&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Tue Apr  2 13:49:41 2019
@@ -468,11 +468,13 @@ llvm::Function *CGNVCUDARuntime::makeReg
 /// \endcode
 llvm::Function *CGNVCUDARuntime::makeModuleCtorFunction() {
   bool IsHIP = CGM.getLangOpts().HIP;
+  bool IsCUDA = CGM.getLangOpts().CUDA;
   // No need to generate ctors/dtors if there is no GPU binary.
   StringRef CudaGpuBinaryFileName = CGM.getCodeGenOpts().CudaGpuBinaryFileName;
   if (CudaGpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
-  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+  if ( (IsHIP || (IsCUDA && !RelocatableDeviceCode) )
+   && EmittedKernels.empty() && DeviceVars.empty())
 return nullptr;
 
   // void __{cuda|hip}_register_globals(void* handle);

Modified: cfe/trunk/test/CodeGenCUDA/device-stub.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/device-stub.cu?rev=357526&r1=357525&r2=357526&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/device-stub.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/device-stub.cu Tue Apr  2 13:49:41 2019
@@ -227,20 +227,14 @@ void hostfunc(void) { kernelfunc<<<1, 1>
 // There should be no __[[PREFIX]]_register_globals if we have no
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
-// CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
+// CUDANOGLOBALS-NOT: @{{.*}} = private constant{{.*}}
 // HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// CUDANOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// NOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// NOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// CUDANOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
-// CUDANOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
-
-// There should be no fat binary functions when no device-code is found for 
HIP.
-// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// HIPNOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
-// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
-// HIPNOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+// NOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
+// NOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
 
 // There should be no constructors/destructors if we have no GPU binary.
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals


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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357526: [HIP-Clang] Fat binary should not be produced for 
non GPU code 2 (authored by aaronenyeshi, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60141?vs=193328&id=193360#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60141

Files:
  cfe/trunk/lib/CodeGen/CGCUDANV.cpp
  cfe/trunk/test/CodeGenCUDA/device-stub.cu


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -468,11 +468,13 @@
 /// \endcode
 llvm::Function *CGNVCUDARuntime::makeModuleCtorFunction() {
   bool IsHIP = CGM.getLangOpts().HIP;
+  bool IsCUDA = CGM.getLangOpts().CUDA;
   // No need to generate ctors/dtors if there is no GPU binary.
   StringRef CudaGpuBinaryFileName = CGM.getCodeGenOpts().CudaGpuBinaryFileName;
   if (CudaGpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
-  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+  if ( (IsHIP || (IsCUDA && !RelocatableDeviceCode) )
+   && EmittedKernels.empty() && DeviceVars.empty())
 return nullptr;
 
   // void __{cuda|hip}_register_globals(void* handle);
Index: cfe/trunk/test/CodeGenCUDA/device-stub.cu
===
--- cfe/trunk/test/CodeGenCUDA/device-stub.cu
+++ cfe/trunk/test/CodeGenCUDA/device-stub.cu
@@ -227,20 +227,14 @@
 // There should be no __[[PREFIX]]_register_globals if we have no
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
-// CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
+// CUDANOGLOBALS-NOT: @{{.*}} = private constant{{.*}}
 // HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// CUDANOGLOBALS: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// NOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// NOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
-// CUDANOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
-// CUDANOGLOBALS: call void @__[[PREFIX]]UnregisterFatBinary
-
-// There should be no fat binary functions when no device-code is found for 
HIP.
-// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// HIPNOGLOBALS-NOT: 
call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
-// HIPNOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
-// HIPNOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
+// NOGLOBALS-NOT: define internal void @__[[PREFIX]]_module_dtor
+// NOGLOBALS-NOT: call void @__[[PREFIX]]UnregisterFatBinary
 
 // There should be no constructors/destructors if we have no GPU binary.
 // NOGPUBIN-NOT: define internal void @__[[PREFIX]]_register_globals


Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -468,11 +468,13 @@
 /// \endcode
 llvm::Function *CGNVCUDARuntime::makeModuleCtorFunction() {
   bool IsHIP = CGM.getLangOpts().HIP;
+  bool IsCUDA = CGM.getLangOpts().CUDA;
   // No need to generate ctors/dtors if there is no GPU binary.
   StringRef CudaGpuBinaryFileName = CGM.getCodeGenOpts().CudaGpuBinaryFileName;
   if (CudaGpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
-  if (IsHIP && EmittedKernels.empty() && DeviceVars.empty())
+  if ( (IsHIP || (IsCUDA && !RelocatableDeviceCode) )
+   && EmittedKernels.empty() && DeviceVars.empty())
 return nullptr;
 
   // void __{cuda|hip}_register_globals(void* handle);
Index: cfe/trunk/test/CodeGenCUDA/device-stub.cu
===
--- cfe/trunk/test/CodeGenCUDA/device-stub.cu
+++ cfe/trunk/test/CodeGenCUDA/device-stub.cu
@@ -227,20 +227,14 @@
 // There should be no __[[PREFIX]]_register_globals if we have no
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
-// CUDANOGLOBALS: @{{.*}} = private constant{{.*}}
+// CUDANOGLOBALS-NOT: @{{.*}} = private constant{{.*}}
 // HIPNOGLOBALS-NOT: @{{.*}} = internal constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// CUDANOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
-// CUDANOGLOBALS: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
+// NOGLOBALS-NOT: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
+// NOGLOBALS-NOT: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PR

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

Hi Artem, I had just committed the change. IS this change OK or should I revert 
it?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60141



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

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

[Don Hinton via Phabricator via cfe-commits]

hintonda created this revision.
hintonda added a reviewer: alexfh.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Fixed ambiguous namespace bug in llvm checkers that add the
llvm namespace to clang::tidy, by adding using declaration for
SmallSet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60151

Files:
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang/include/clang/Basic/LLVM.h


Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(


Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60112: [analyzer] Treat write into a top-level parameter variable with destructor as escape.

[Artem Dergachev via Phabricator via cfe-commits]

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

In D60112#1451198 , @Szelethus wrote:

> Woah, the code looks amazing, cheers on the refactoring! I'll be honest, I'm 
> struggling a bit with the sentence "we're now in the top frame". In order, I 
> don't understand what does
>
> - we
> - now
> - in the top frame mean. "Top-level argument" is another one -- Do we have 
> **precise** definitions for there terms?


Cf. `LocationContext::inTopFrame()`.

The top frame is the `StackFrameContext` whose parent context is `nullptr`. 
There is only one such context and it is the root of the location context tree. 
It corresponds to the stack frame from which the current Analysis begins. That 
is, each such context corresponds to a path-sensitive analysis line in the 
`-analyzer-display-progress` dump. Other stack frame contexts (with non-null 
parents) correspond to function calls *during* analysis. They usually 
correspond to stack frames of inlined calls (since D49443 
 we sometimes create stack frame contexts for 
calls that will never be inlined, but the rough idea is still the same).

So this patch talks about the situation when we start analysis from function, 
say, `test(A a) { ... }`, and its argument `a` exists throughout the whole 
analysis: its constructor was called before the analysis has started, and its 
destructor will be called after the analysis has ended. Having no parent 
context means that we don't know anything about the caller or the call site of 
`test(a)` - the information we usually do have for inlined calls.

The phrase "We are now in the top frame" therefore roughly translates to "The 
`CoreEngine` worklist item that the `ExprEngine` is currently processing 
corresponds to an `ExplodedNode` that has a `ProgramPoint` with a 
`LocationContext` whose nearest `StackFrameContext` is the top frame". When i'm 
talking about top-level variables, i mean "A `VarRegion` whose parent region is 
a `StackArgumentsSpaceRegion` whose identity is a top-frame 
`StackFrameContext`".

Do you think i should document it somehow?




Comment at: clang/test/Analysis/malloc.cpp:151
+  char *getName() {
+if (!name) {
+  name = static_cast(malloc(10));

Szelethus wrote:
> Is this relevant? `name` will never be null.
Not really, just makes the code look a bit more sensible and idiomatic and less 
warning-worthy-anyway, to make it as clear as possible that the positive here 
is indeed false. We don't really have a constructor in this class, but we can 
imagine that it zero-initializes name. Without this check calling `getName()` 
multiple times would immediately result in a leak.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60112



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

[PATCH] D60141: [HIP-Clang] Fat binary should not be produced for non GPU code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D60141#1452019 , @ashi1 wrote:

> Hi Artem, I had just committed the change. IS this change OK or should I 
> revert it?


The `if` condition could use some clang-formatting, but other than that the 
patch still looks OK to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60141



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