[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + lebedev.ri wrote: > aaron.ballman wrote: > > Why 2, 10, and 100? > These really should be a config option. T

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

jgalenson added a comment. In https://reviews.llvm.org/D49492#1167064, @efriedma wrote: > Are you sure this will actually do what you want, in general? I suspect it > will end up missing bounds checks in some cases because it's running it too > early (before mem2reg/inlining/etc). No, I'm no

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

efriedma added a comment. skipRValueSubobjectAdjustments has to match the rules in `[class.temporary]` in the standard, which includes skipping over certain explicit casts. Would it be enough to accumulate the skipped casts into a SmallVector, like we do for the skipped comma operators?

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

eugenis added a comment. ObjectSizeOffsetEvaluator may fail to trace the address back to the underlying object, and we will end up not inserting the check. Does ChecksUnable statistic go up with this change? Repository: rC Clang https://reviews.llvm.org/D49492

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

efriedma added a comment. This sanitizer has a bit of a strange design compared to other sanitizers; it tries to compute the size of the base object using the IR at the point the pass runs. So the later it runs, the more information it has. Trivial example: static int accumulate(int* foo, i

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

leonardchan created this revision. leonardchan added reviewers: phosek, mcgrathr. leonardchan added a project: clang. This patch adds the `noderef` attribute in clang and checks for dereferences of types that have this attribute. This attribute is currently used by sparse and would like to be po

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

eugenis added a comment. Some checks can be removed by asking SCEV for offset bounds, see SafeStack::IsAccessSafe for an example. Repository: rC Clang https://reviews.llvm.org/D49492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

dankm added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:609 static void addDebugPrefixMapArg(const Driver &D, const ArgList &Args, ArgStringList &CmdArgs) { + for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) { +StringRef Map = A->getVal

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + aaron.ballman wrote: > lebedev.ri wrote: > > aaron.ballman wrote: > > > Why 2, 10, and 100? > > These really s

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

dankm updated this revision to Diff 156164. dankm retitled this revision from "Initial implementation of -fmacro-prefix-map and -ffile-prefix-map" to "Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map". dankm added a comment. Address some of the comments by erichkeane and joerg.

[PATCH] D49476: [Index] Set OrigD before D is changed.

akyrtzi added a comment. Is it possible to add a regression test case ? I assume this is fixing some issue, we should make sure we don't regress again. Repository: rC Clang https://reviews.llvm.org/D49476 ___ cfe-commits mailing list cfe-commits

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

jgalenson added a comment. Good call; I had figured that running it earlier might just impede other optimizations, but I forgot that it would also hide size information. I thought this was the easiest approach compared to changing the pass pipeline or adding extra checks in here. But I hadn't

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

ldionne added a comment. ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is not supported, we're already marking the base template as `__visibility__("default")`, so marking the extern template declaration with `__visibility__("default")` is not a problem. If `__type_visib

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

leonardchan updated this revision to Diff 156167. leonardchan added a comment. - Added checks for expressions in statements other than declarations or expression statements Repository: rC Clang https://reviews.llvm.org/D49511 Files: include/clang/AST/Type.h include/clang/Basic/Attr.td

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

NoQ added a comment. Herald added a subscriber: mikhail.ramalho. There are still performance regressions coming in, and this time it doesn't look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208 I suspect that this might be because we aren't enforcing complexity thresholds over a

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

thomasanderson added a comment. In https://reviews.llvm.org/D35388#1167305, @ldionne wrote: > ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is not > supported, we're already marking the base template as > `__visibility__("default")`, so marking the extern template declar

[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant

leonardchan updated this revision to Diff 156174. leonardchan marked 3 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D48661 Files: include/clang/AST/ASTContext.h include/clang/Basic/FixedPoint.h include/clang/Basic/TargetInfo.h lib/AST/ASTContext.cpp lib/Basi

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

arphaman added a comment. In https://reviews.llvm.org/D48559#1165511, @sammccall wrote: > In https://reviews.llvm.org/D48559#1165172, @jkorous wrote: > > > Hi Sam, thanks for your feedback! > > > > In general I agree that explicitly factoring out the transport layer and > > improving layering wo

[PATCH] D49303: [CodeGen][ObjC] Treat non-escaping blocks as global blocks to make copy/dispose a no-op

ahatanak planned changes to this revision. ahatanak added a comment. In https://reviews.llvm.org/D49303#1165902, @rjmccall wrote: > Please test that we still copy captures correctly into the block object and > destroy them when the block object is destroyed. There is a bug in my patch where th

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

vsapsai added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1875 + // Stop further preprocessing if a fatal error has occurred. Any diagnostics + // we might have raised will not be visible. + if (ShouldEnter && Diags->hasFatalErrorOccurred()) jk

r337430 - Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific LangOpts

Author: bruno Date: Wed Jul 18 16:21:19 2018 New Revision: 337430 URL: http://llvm.org/viewvc/llvm-project?rev=337430&view=rev Log: Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific LangOpts Summary: Reproducer and errors: https://bugs.llvm.org/show_bug.cgi?id=37878 look

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai created this revision. vsapsai added reviewers: bruno, benlangmuir. Herald added a subscriber: dexonsmith. Orphaned files prevent us from building a file system tree and cause the assertion > Assertion failed: (NewParentE && "Parent entry must exist"), function > uniqueOverlayTree, file

r337431 - Documentation: fix a typo in the AST Matcher Reference docs.

Author: jdennett Date: Wed Jul 18 16:21:31 2018 New Revision: 337431 URL: http://llvm.org/viewvc/llvm-project?rev=337431&view=rev Log: Documentation: fix a typo in the AST Matcher Reference docs. Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Modified: cfe/trunk/include/clang/AS

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai added inline comments. Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1416 +if (NameValueNode) + error(NameValueNode, "file is not located in any directory"); +return nullptr; Not happy with the error message but didn't come up

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

ldionne added a comment. In https://reviews.llvm.org/D35388#1167315, @thomasanderson wrote: > In https://reviews.llvm.org/D35388#1167305, @ldionne wrote: > > > ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is > > not supported, we're already marking the base template as

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.h:52 + + const std::vector IngnoredValuesInput; + std::vector IngnoredValues; `Ignored`. Please spellcheck throughout. Comment at: docs/clang-tidy/checks/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 17 inline comments as done. 0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32 + for (const std::string &IgnoredValue : IngnoredValuesInput) { +IngnoredValues.push_back(std::stoll(IgnoredValue)); + }

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

erik.pilkington updated this revision to Diff 156178. erik.pilkington marked 5 inline comments as done. erik.pilkington added a comment. This revision is now accepted and ready to land. In this new patch: - Add support for __builtin_bzero (-Wsuspicious-bzero), improve diagnostics for platforms t

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:7984 + if (isa(ThirdArg) && + cast(ThirdArg)->getValue() == 0) { +WarningKind = 0; Quuxplusone wrote: > > Suppress the diagnostic in cases like `memset(ptr, 0xff, PADDING

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman created this revision. arphaman added reviewers: jkorous, sammccall, ilya-biryukov. arphaman added a project: clang-tools-extra. Herald added subscribers: dexonsmith, MaskRay, ioeric. This patch builds on top of the "extra flags" extension added in r307241. It adds the ability to specify

[PATCH] D49523: [clangd] Add support for per-file override compilation command

arphaman added a comment. Btw, the "extraFlags" extension is still usable with "compilationCommand". Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

yaxunl updated this revision to Diff 156184. yaxunl marked 4 inline comments as done. yaxunl added a comment. Added comments about thread safety of ctor functions. https://reviews.llvm.org/D49083 Files: lib/CodeGen/CGCUDANV.cpp test/CodeGenCUDA/device-stub.cu Index: test/CodeGenCUDA/device

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 8 inline comments as done. 0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:33 + + return -3; // FILENOTFOUND + } Quuxplusone wrote: > I notice you changed `-1` to `-3`. Is `return -1` n

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

ahatanak added a comment. ping Repository: rC Clang https://reviews.llvm.org/D45898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. I think this requires a separate discussion - do we accept magic values only when they are used directly to initialize a constant of the same type? Or do we allow them generically when they are used to initialize any constant? Or do we only accept several layers of

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

emmettneyman created this revision. emmettneyman added reviewers: morehouse, kcc. Herald added subscribers: cfe-commits, mgorny. Herald added a reviewer: alexshap. Made changes to the llvm-proto-fuzzer - Added loop vectorizer optimization pass in order to have two IR versions - Updated old fuzz t

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

emmettneyman added a comment. The files Object.h Object.cpp llvm-objcopy.h are from llvm/tools/llvm-obj-copy with only slight modifications, mostly deleting irrelevant parts. Repository: rC Clang https://reviews.llvm.org/D49526 ___ cfe-c

r337433 - [clang]: Add support for "-fno-delete-null-pointer-checks"

Author: manojgupta Date: Wed Jul 18 17:44:52 2018 New Revision: 337433 URL: http://llvm.org/viewvc/llvm-project?rev=337433&view=rev Log: [clang]: Add support for "-fno-delete-null-pointer-checks" Summary: Support for this option is needed for building Linux kernel. This is a very frequently reque

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

This revision was automatically updated to reflect the committed changes. Closed by commit rC337433: [clang]: Add support for "-fno-delete-null-pointer-checks" (authored by manojgupta, committed by ). Changed prior to commit: https://reviews.llvm.org/D47894?vs=155891&id=156195#toc Repository:

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

pcc added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:209 + +// Helper function that converts ELF relocatable into raw machine code that +// can be executed in memory. Returns size of machine code. Did you look at using LLVM'

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

phosek updated this revision to Diff 156201. phosek marked 2 inline comments as done. Repository: rCXX libc++ https://reviews.llvm.org/D49502 Files: libcxx/CMakeLists.txt libcxx/cmake/Modules/HandleLibCXXABI.cmake libcxx/lib/CMakeLists.txt libcxxabi/CMakeLists.txt libcxxabi/src/CMake

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

phosek added inline comments. Comment at: libcxx/lib/CMakeLists.txt:269 +AND (TARGET cxxabi_static OR HAVE_LIBCXXABI)) +#if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR +#(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI)) ---

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

vsapsai added a comment. Need to double check what tests we have when using relative path names at the root level. I'd like to make the behavior consistent because a file name is a specific case of relative paths. So far there are no assertions and no errors but file lookup doesn't seem to be w

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156203. 0x8000- added a comment. Herald added subscribers: kbarton, nemanjai. Significant refactoring to address review comments - mainly to reduce duplication and implement in functional style. cppcoreguidelines-avoid-magic-numbers is documented as

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

0x8000- added a comment. Any plans for fix-its that will add the suggested 'const' qualifiers? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: test/clang-tidy/readability-magic-numbers.cpp:16 +void BuggyFunction() { + int BadLocalInt = 6; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 [readabili

[PATCH] D49523: [clangd] Add support for per-file override compilation command

malaperle added a comment. Interesting! We also have a need for passing compilation commands in a context where there is no compile_commands.json, but we were thinking of putting this in a "didChangeConfiguration" message so that all the commands would be available even before files are opened.

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

rjmccall added a comment. Thanks for the comment. Comment at: lib/CodeGen/CGCUDANV.cpp:444 +auto HandleValue = +CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign()); +llvm::Constant *Zero = llvm::Constant::getNullValue(HandleValue->getType());

[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant

rjmccall added inline comments. Comment at: include/clang/Basic/FixedPoint.h:64 + +class APFixedPoint { + public: rjmccall wrote: > This should get a documentation comment describing the class. You should > mention that, like `APSInt`, it carries all of the sem

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

rjmccall added a comment. I think it would be reasonable to set a flag on `ImplicitCastExpr`s that are actually semantically part of an explicit cast. I don't think that would be hard to get Sema to do, either by passing a flag down to the code that builds those casts or just by retroactively

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

Quuxplusone updated this revision to Diff 156210. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Remove some incorrect `noexcept` from ``. I noticed this because the calls to `__clang_call_terminate` showed up on Godbolt. But the pessimization is still there even w

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

lebedev.ri added a comment. In https://reviews.llvm.org/D49508#1167177, @efriedma wrote: > skipRValueSubobjectAdjustments has to match the rules in `[class.temporary]` > in the standard, which includes skipping over certain explicit casts. I'm this approach because this is what @rsmith suggest

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

gaijiading added a comment. In https://reviews.llvm.org/D46230#1167023, @echristo wrote: > In https://reviews.llvm.org/D46230#1166958, @gaijiading wrote: > > > In https://reviews.llvm.org/D46230#1166919, @echristo wrote: > > > > > LGTM. > > > > > > -eric > > > > > > Hi Eric, I do not have commit

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

baloghadamsoftware added a comment. In https://reviews.llvm.org/D41938#1167313, @NoQ wrote: > There are still performance regressions coming in, and this time it doesn't > look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208 > > I suspect that this might be because we aren't enfo

<    1   2