[PATCH] D45719: [clang-Format] Fix indentation of member call after block

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added inline comments.



Comment at: lib/Format/FormatToken.h:323
+  bool closesScopeAfterBlock() const {
+if(BlockKind == BK_Block)
+  return true;

Looks like all other `if`s in this file have space before `(`.


Repository:
  rC Clang

https://reviews.llvm.org/D45719



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

[PATCH] D52360: [clang-tidy] Fix for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added a comment.

Would that also skip checks for something like `shared_ptr`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52360



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

[PATCH] D62574: Initial draft of target-configurable address spaces.

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added a comment.

What are the remaining roadblocks left before this patch can be merged? I'm 
interested in having a target-specific way to define the allowed 
explicit/implicit address space conversions.
Also, it appears that currently whether implicit casts between pointers of 
different AS are allowed is determined by the superset relations only, however 
https://reviews.llvm.org/D73360 introduced another (openCL-specific) variable 
into the mix: whether the conversion is a top level or not. IMHO, this should 
also be configured by the target, although I'm not sure whether it needs a 
separate hook (isBitcastNoop or similar?) or it needs to check the legality of 
the implicit casts differently.


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

https://reviews.llvm.org/D62574



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

[PATCH] D62574: Initial draft of target-configurable address spaces.

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added a comment.

In D62574#2135662 , @ebevhan wrote:

> It's generally not safe to alter address spaces below the top level. C is 
> just very permissive about it.


Isn't that also true for the top-level casts? Unless when it is. And the target 
should know when it's safe or not. It's just that for non top-level casts there 
is added condition that casts are noop (i.e. don't change the bit pattern of 
the pointer). At least that's how I see it.

I think that you are write about that patch being targeted at C++ (need to do 
some C tests), but having the same implicit conversion being legal in C and not 
in C++ would be a bit confusing for the users (especially when the target knows 
it's completely safe). Anyway, this is not as important as getting this 
target-configurable AS work done. Without it, I guess the only option for 
downstream works to customize the behavior is to introduce their own 
LangAS-level AS entries, which is definitely not ideal.


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

https://reviews.llvm.org/D62574



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

[PATCH] D89220: [clang-rename] Simplify the code of handling class paritial specializations, NFC.

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added inline comments.



Comment at: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp:119
+llvm::for_each(PartialSpecs,
+   [&](const auto *Spec) { addUSRsOfCtorDtors(Spec); });
 addUSRsOfCtorDtors(TemplateDecl->getTemplatedDecl());

Btw, this breaks on GCC <= 7.1, due to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67274
Workaround is to use `this->`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89220

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

[PATCH] D62574: Add support for target-configurable address spaces.

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added a comment.

So is missing an in-tree user (like AMDGPU) the only thing that prevents from 
merging this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62574

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

[PATCH] D155955: [Clang] Improve the handling of large arrays evaluation.

[Danila Malyutin via Phabricator via cfe-commits]

danilaml added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:1026
+  // APValue stores array extents as unsigned,
+  // so anything that is greater that unsigned would overflow when
+  // constructing the array, we catch this here.





Comment at: clang/lib/AST/ExprConstant.cpp:1027
+  // so anything that is greater that unsigned would overflow when
+  // constructing the array, we catch this here.
+  if (BitWidth > ConstantArrayType::getMaxSizeBits(Ctx) ||




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155955

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