[PATCH] D68093: [clang-scan-deps][static analyzer] Support for clang --analyze in scan-deps

This revision was automatically updated to reflect the committed changes. Closed by commit rGc5d14b5c6fa9: [clang-scan-deps] Support for clang --analyze in clang-scan-deps (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Mo

[PATCH] D69017: Include sanitize blacklist and other extra deps as part of scan-deps output

jkorous accepted this revision. jkorous added a comment. Looks great! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69017/new/ https://reviews.llvm.org/D69017 ___ cfe-commits mailing list cfe-c

[PATCH] D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output

jkorous added a comment. I think you could've just used `CHECK-DAG` to fix the tests. It *might* be a bit more robust. Although just reordering checks seems perfectly fine too. https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive Using `std::set` here sounds reasonable to me

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

jkorous added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:70 +}; +if (const int RC = llvm::sys::ExecuteAndWait(ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) { + auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str()); -

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80279/new/ https://reviews.llvm.org/D80279 _

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

jkorous added a comment. Hi Owen, Do you plan to land the functionality for emitting documentation URLs in clang too? Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323 + // A documentation URL has an ID and path size. + if (Record.size() != 2) +

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

This revision was automatically updated to reflect the committed changes. Closed by commit rGf7c7e8a523f5: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

jkorous added a comment. In D77177#2049805 , @thakis wrote: > This breaks the build everywhere (e.g. > http://45.33.8.238/linux/18283/step_4.txt) and has been in for over an hour. > Please watch bots after landing. (I think changes that are created more

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

jkorous added inline comments. Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323 + // A documentation URL has an ID and path size. + if (Record.size() != 2) +return SDError::MalformedDiagnosticRecord; owenv wrote: > owenv wrote:

[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG660cda572d6e: [Analyzer][WebKit] NoUncountedMembersChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D79063: [ASTMatchers] Matchers related to C++ inheritance

This revision was automatically updated to reflect the committed changes. Closed by commit rG1a5c97f3a4b8: [ASTMatchers] Matchers related to C++ inheritance (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

jkorous added a comment. Hi @kadircet! I am investigating a failure of `PatchesAdditionalIncludes` test that we got internally. It's a failed assert in `ReplayPreamble::replay`. Our clangd source code is for all practical purposes identical to upstream version but not so with clang source. Speci

[PATCH] D81017: [Analyzer][WebKit] Check record definition is available in NoUncountedMembers checker

This revision was automatically updated to reflect the committed changes. Closed by commit rGd61ad660503d: [Analyzer][WebKit] Check record definition is available in NoUncountedMembers… (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D77612: [ASTMatchers] Fix isDerivedFrom for recursive templates

This revision was automatically updated to reflect the committed changes. Closed by commit rG14d89bfbe0b4: [ASTMatchers] Fix isDerivedFrom for recursive templates (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://revie

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added a comment. @njames93 `hasDirectBase` seems like a useful matcher to me! OTOH I am not totally convinced about `hasType` -> `hasClass`. Anyway, don't you want to land `hasDirectBase` as a separate patch first and then discuss the rest? One more thing - I'm just thinking if there is

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3621 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClassOrClassTemplate, + internal::Matcher, InnerMatcher) { I think we should just use `eachOf` matcher for

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { aaron.ballman wrote: > jkorous wrote: > > Nit: while "[base sp

[PATCH] D81691: [clangd] Set CWD in semaCodeComplete

jkorous added a comment. A slightly tangential thing - we recently got an internal bugreport about clangd handling combination of working directory and relative paths for `-fmodules-cache-path` combination in compile_command.json incorrectly. I tried some quick hacks but failed - it seems that u

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { aaron.ballman wrote: > njames93 wrote: > > jkorous wrote: > >

[PATCH] D77179: [Analyzer][WebKit] UncountedCallArgsChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rGa7eb3692e762: [Analyzer][WebKit] UncountedCallArgsChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM! Thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82629/new/ https://reviews.llvm.org/D82629

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. @milianw I just approved https://reviews.llvm.org/D82629 - I feel like that patch is addressing the core issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82740/new/ https://reviews.llvm.org/D82740 __

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

jkorous added a comment. @ckandeler do you have commit access or do you want me to land the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82629/new/ https://reviews.llvm.org/D82629 ___ cfe-commi

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6e089e98a9d5: [libclang] Fix crash when visiting a captured VLA (authored by ckandeler, committed by jkorous). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D83438: [AST] Fix potential nullptr dereference in Expr::HasSideEffects

This revision was automatically updated to reflect the committed changes. Closed by commit rGfdb69539bcd2: [AST] Fix potential nullptr dereference in Expr::HasSideEffects (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Mon

[PATCH] D83438: [AST] Fix potential nullptr dereference in Expr::HasSideEffects

jkorous added a comment. Sounds like a good idea. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83438/new/ https://reviews.llvm.org/D83438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. @milianw could you try to reduce the reproducer you have? Maybe take lldb and see where does the `nullptr` come from? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82740/new/ https://reviews.llvm.org/D82740

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. Your patch definitely fixes the symptoms of the bug. I just want to make sure that we aren't covering some logic error with a bandaid as it would be harder to find out the root cause once we land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78760: Check a class has a definition before iterating over its base classes

jkorous added a comment. In D78760#2003118 , @rsmith wrote: > If so, we'll need to make sure that all code that iterates over base classes > checks for this condition (I bet there are more cases than the two that you > found). Just a potentially relate

[PATCH] D82837: [Analyzer][WebKit] UncountedLambdaCaptureChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG820e8d8656ec: [Analyzer][WebKit] UncountedLambdaCaptureChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D79451: [libclang] Remove duplicate dependency on LLVMSupport

This revision was automatically updated to reflect the committed changes. Closed by commit rG02b303321d3f: [libclang] Remove duplicate dependency on LLVMSupport (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHA

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

jkorous added a comment. IIUC the issue is that `SourceManager::translateFile()` basically consists of two blocks of code: // First, check the main file ID, since it is common to look for a // location in the main file. if (MainFileID.isValid()) { bool Invalid = false; const SLocEn

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

jkorous added a comment. Could you please add a test for this method to `clang/unittests/Basic/SourceManagerTest.cpp`? Comment at: clang/include/clang/Basic/SourceManager.h:816 + /// Returns true when the given FileEntry corresponds to the main file. + bool isMainFile(File

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM! Thanks for the work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100 ___

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM! Thanks Alex! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79834/new/ https://reviews.llvm.org/D79834 ___ cfe-commits mailing li

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

This revision was automatically updated to reflect the committed changes. Closed by commit rG05eedf1f5b44: [clang][VerifyDiagnosticConsumer] Support filename wildcards (authored by arames, committed by jkorous). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D39730: Enabling constructor code completion

jkorous-apple added a comment. Sorry for delays, I'm working on this on and off when there's nothing more important. Let me specify a little narrower scope of this change: - Make code completion contain constructor item for out-of-line constructor definition: struct foo { foo(); }; foo:: // re

[PATCH] D39279: Stringizing raw string literals containing newline

jkorous-apple added inline comments. Comment at: lib/Lex/Lexer.cpp:217 +void StringifyImpl(T& Str, char Quote) { + unsigned i = 0, e = Str.size(); + while (i < e) { Wouldn't **auto** or **typename T::size_type** instead of **unsigned** be more appropriate here

[PATCH] D39730: Enabling constructor code completion

jkorous-apple added a comment. No luck. If I understand it correctly all the information that would be needed to distinguish between: foo:: and int i = foo:: in ResultBuilder::AddResult() is missing since the context is represented only by DeclContext. Something like a DirectASTParentNod

[PATCH] D39279: Stringizing raw string literals containing newline

jkorous-apple added a comment. I am sorry I wasn't really clear. What I meant was to do something like this (pseudo-code, dealing only with newlines): if( Str.size() == 0) return; // Calculate all the extra space needed first. typename T::size_type extra_space = 0; bool previou

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

jkorous-apple added inline comments. Comment at: tools/libclang/CIndexer.cpp:131 + if (!File.empty()) +llvm::sys::fs::remove(File); +} Just a thought - since we are not propagating errors from constructor we are not really sure we were able to create the fi

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

jkorous-apple added inline comments. Comment at: tools/libclang/CIndexer.cpp:108 + // Write out the information about the invocation to it. + auto WriteStringKey = [&](StringRef Key, StringRef Value) { +OS << R"(")" << Key << R"(":")"; Nit: Maybe capturing

[PATCH] D39279: Stringizing raw string literals containing newline

jkorous-apple added a comment. Thank you for your patience @twoh and sorry for the delay. I have few suggestions about doxygen annotations but otherwise LGTM. Comment at: include/clang/Lex/Lexer.h:247 + /// add surrounding ""'s to the string. If Charify is true, this escapes t

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

jkorous-apple added a comment. LGTM otherwise. Comment at: tools/libclang/CIndexer.h:45 + std::string ToolchainPath; + llvm::sys::SmartMutex Mutex; + I am just wondering - since we anticipate multi-threaded usage, shouldn't we synchronize access to all membe

[PATCH] D39279: Stringizing raw string literals containing newline

jkorous-apple added a comment. I see your concern. Would you just consider moving the annotation to header file then? It looks more like documentation of public interface than implementation details to me. I imagine any programmer interested in these methods needs to know these details so they

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. In D130055#3683173 , @aaron.ballman wrote: > Are there circumstances where we cannot "simply" infer this from the > constructor itself? (After instantiation) we know the class hierarchy, we > know the data members, and we know

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. In D130055#3683279 , @beanz wrote: > I'm wondering if there could be a generalization of the attribute like: I can't imagine it is possible to design a reasonable and practically usable system of attributes to model anything but

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. One more thing - the "attributes as a cross-TU metadata" idea might be (possibly with some squinting) conceptually similar to `enforce_tcb` attribute that @NoQ made me aware of some time ago. https://clang.llvm.org/docs/AttributeReference.html#enforce-tcb Repository:

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. Hi Chris! This is a very interesting idea! I do have couple thoughts - mostly that this could lead to something great and I would love it to apply to as many relevant cases as possible. It looks like there is a possibility that a free function, static method or a metho

[PATCH] D123273: [utils] Avoid hardcoding metadata ids in update_cc_test_checks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce583b14b2ec: [utils] Avoid hardcoding metadata ids in update_cc_test_checks (authored by jkorous). Herald added a project: clang. Herald added a sub

[PATCH] D123273: [utils] Avoid hardcoding metadata ids in update_cc_test_checks

jkorous added a comment. Reverting for now. Will take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123273/new/ https://reviews.llvm.org/D123273 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

jkorous added a comment. In D125349#3509546 , @ahatanak wrote: > Is it not possible to handle this similarly to `volatile unsigned`? If I > replace `_Atomic unsigned` with `volatile unsigned`, I see > `LookupOverloadedBinOp` succeed without having to st

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

jkorous added a comment. In D125349#3509073 , @aaron.ballman wrote: > It's interesting to note that `an_atomic_uint = an_atomic_uint + > an_enum_value` works correctly: https://godbolt.org/z/cvP9e6nh7. I was trying > to figure out whether the atomic qu

[PATCH] D66555: [driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation

jkorous added inline comments. Comment at: clang/lib/Driver/Driver.cpp:1044 + if (const Arg *A = Args.getLastArg(options::OPT_gen_cdb_fragment_path)) +PathToCDBFragmentDir = A->getValue(); // FIXME: TargetTriple is used by the target-prefixed calls to as/ld --

[PATCH] D66555: [driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation

jkorous added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.h:99 + void DumpCompilationDatabaseFragmentToDir( + StringRef Dir, Compilation &C, StringRef Target, const InputInfo &Output, + const InputInfo &Input, const llvm::opt::ArgList &Args) const; -

[PATCH] D66555: [driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation

jkorous added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2067 + const auto &Driver = C.getDriver(); + auto CWD = Driver.getVFS().getCurrentWorkingDirectory(); + if (!CWD) { Do we still need this now? Comment at: clang

[PATCH] D66694: [libclang][index][NFCi] Refactor machinery for skipping already parsed function bodies

This revision was automatically updated to reflect the committed changes. Closed by commit rGb1c90791024b: [libclang][index][NFCi] Refactor machinery for skipping function bodies (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Gi

[PATCH] D66694: [libclang][index][NFCi] Refactor machinery for skipping already parsed function bodies

jkorous marked 4 inline comments as done. jkorous added a comment. Hmm, I already landed this - let's transfer the discussion to https://reviews.llvm.org/D66764. I'll make the changes to that patch. Is that ok? Comment at: clang/tools/libclang/Indexing.cpp:126 +/// Is thread-s

[PATCH] D66694: [libclang][index][NFCi] Refactor machinery for skipping already parsed function bodies

jkorous marked 3 inline comments as done. jkorous added inline comments. Comment at: clang/tools/libclang/Indexing.cpp:371 - SessionSkipBodyData *SKData; - std::unique_ptr SKCtrl; + SharedParsedRegionsStorage *SKData; + std::unique_ptr ParsedLocsTracker; jk

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

jkorous added inline comments. Comment at: clang/lib/Index/IndexingAction.cpp:77 +IndexCtx->getDataConsumer().setPreprocessor(PP); +PP->addPPCallbacks(std::make_unique(IndexCtx)); + } gribozavr wrote: > ilya-biryukov wrote: > > The fact that we call `add

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

jkorous added inline comments. Comment at: clang/include/clang/Index/IndexingAction.h:60 + +inline std::unique_ptr createIndexingASTConsumer( +std::shared_ptr DataConsumer, Why not use default argument instead of overloading? Repository: rG LLVM Github Mo

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

jkorous added a comment. Otherwise LGTM. Thanks for refactoring this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66879/new/ https://reviews.llvm.org/D66879 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D66879: [Index] Added a ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it instead of inventing their own solution

jkorous added inline comments. Comment at: clang/include/clang/Index/IndexingAction.h:60 + +inline std::unique_ptr createIndexingASTConsumer( +std::shared_ptr DataConsumer, gribozavr wrote: > jkorous wrote: > > Why not use default argument instead of overload

[PATCH] D67077: [libclang] Refactored SharedParsedRegionsStorage

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Thanks for polishing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67077/new/ https://reviews.llvm.org/D67077 ___

[PATCH] D72409: [clang] Fix out-of-bounds memory access in ComputeLineNumbers

This revision was automatically updated to reflect the committed changes. Closed by commit rGf28972facc1f: [clang] Fix out-of-bounds memory access in ComputeLineNumbers (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https:/

[PATCH] D86231: [SourceManager] Explicitly check for potential iterator underflow

This revision was automatically updated to reflect the committed changes. Closed by commit rGae726fecae9a: [SourceManager] Explicitly check for potential iterator underflow (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github M

[PATCH] D88133: [Analyzer][WebKit] Use tri-state types for relevant predicates

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG47e6851423fd: [Analyzer][WebKit] Use tri-state types for relevant predicates (authored by jkorous). Herald added a project

[PATCH] D88133: [Analyzer][WebKit] Use tri-state types for relevant predicates

jkorous added a comment. Created this for eventual post-commit review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88133/new/ https://reviews.llvm.org/D88133 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D66854: [index-while-building] PathIndexer

This revision was automatically updated to reflect the committed changes. Closed by commit rG4da126c3748f: [index-while-building] PathIndexer (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D66854?vs

[PATCH] D86991: [libclang] Expose couple AST details

This revision was automatically updated to reflect the committed changes. Closed by commit rG052f83890349: [libclang] Expose couple more AST details via cursors (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHA

[PATCH] D86992: [libclang] Expose Rewriter in libclang API

This revision was automatically updated to reflect the committed changes. Closed by commit rG69e5abb57b70: [libclang] Add CXRewriter to libclang API (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D8

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

jkorous added subscribers: dexonsmith, akyrtzi. jkorous added a comment. Hi @usaxena95 and @sammccall, I am wondering about couple high-level things. Do you guys intend to open-source also the training part of the model pipeline or publish a model trained on generic-enough training set so it co

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

jkorous added a comment. Thanks for working on this! Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2907 +// start with it. +llvm::SmallVector Stack{DynTypedNode::create(*BugStmt)}; + Since IIUC a node can have multiple parents - does that me

[PATCH] D87176: [clang] Enable selectively turning on/off format-insufficient-args warning

This revision was automatically updated to reflect the committed changes. Closed by commit rG1e86d637eb4f: [clang] Selectively ena/disa-ble format-insufficient-args warning (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github M

[PATCH] D87176: [clang] Enable selectively turning on/off format-insufficient-args warning

jkorous added a comment. Had to fix the order of expected warnings in warning-wall.c 6fd8c69049a8 [clang] Update warning-wall.c test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D86230: [SourceManager] Skip module maps when searching files for macro arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rGe7870223d8b5: [SourceManager] Skip module maps when searching files for macro arguments (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commi

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

jkorous added a comment. Friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89959/new/ https://reviews.llvm.org/D89959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

jkorous requested changes to this revision. jkorous added a comment. This revision now requires changes to proceed. Hi! Thank you for the clean-up :) I feel there might be a bit of work still left. While renaming filenames and function names should be mostly inconsequential renaming command line

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

jkorous accepted this revision. jkorous added a comment. LGTM if we have test coverage for all the cases. Comment at: clang/test/CodeGenObjC/category-class-empty.m:1 +// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | FileCheck %s +// PR7431 ---

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

jkorous added a comment. LGTM. Adding the comment would be great. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97878/new/ https://reviews.llvm.org/D97878 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGen/address-safety-attr.cpp:9 -// RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist -// RUN: %clang_cc1 -std=c++11 -triple x86_64-

[PATCH] D133815: [analyzer] Support implicit parameters in path note

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG85d97aac80b8: [analyzer] Support implicit parameter 'self' in path note (authored by jkorous). Herald added a project: clang. Herald added a subscrib

[PATCH] D126908: [VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

jkorous added a comment. Thank you for the patch! Comment at: clang/test/SemaCXX/references.cpp:93 -struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}} +struct C : B, A { }

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. Thanks for the rebase! Nit: I'd just replace `std::function` with `auto` as it saves us repeating the parameter types (and `#include `). Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:617 + // Printers that print extent into OS and sets ExtKnow

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:46 +/// The text indicating that the user needs to provide input there: +constexpr static const char *const UserFillPlaceHolder = "..."; } // end namespace clang

[PATCH] D136811: WIP: RFC: NFC: C++ Buffer Hardening user documentation.

jkorous added a comment. Thank you for the feedback Aaron! We really appreciate the effort you put into this! Comment at: clang/docs/SafeBuffers.rst:69-70 +containers such as ``std::span``, you can achieve bounds safety by +*simply writing good modern C++ code*. This is what t

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added a comment. Thank you for the feedback Gábor! Comment at: clang/docs/SafeBuffers.rst:36-37 +hardened mode where C++ classes such as ``std::vector`` and ``std::span``, +together with their respective ``iterator`` classes, are protected +at runtime agains

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added inline comments. Comment at: clang/docs/SafeBuffers.rst:173 + #pragma unsafe_buffer_usage begin + +Such pragmas not only enable incremental adoption with much smaller granularity, aaron.ballman wrote: > jkorous wrote: > > aaron.ballman wrote: > >

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added a comment. @aaron.ballman We'd like to start making progress on the implementation in parallel to iterating on the documentation and this is our first patch: https://reviews.llvm.org/D137346 Since we'll have about 4 people working full-time on this it isn't reasonable to expect yo

[PATCH] D137346: -Wunsafe-buffer-usage: Initial commit - Transition away from raw buffer accesses.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. @aaron.ballman Do you have any objection to us landing this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137346/new/ https://reviews.llvm.org/D13734

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:127 +static auto isInUnspecifiedLvalueContext(internal::Matcher innerMatcher) { + auto isLvalueToRvalueCast = [](internal::Matcher M) { +return implicitCastExpr(hasCastKind(CastKind::CK_LVal

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. I am sorry I haven't notice this earlier - let's fix this before we land the patch. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:690 + Val.toString(Txt, 10, true); + return Txt.data(); +} We either need a zero to terminate th

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. I got a test failure in `SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp` which I believe is caused solely by the fact that we emit the diagnostics in different order. I am not sure it matters and since the Fix-Its clearly specify what source lines they apply

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:894 +if (VD->getType()->isPointerType()) + return fixVariableWithSpan(VD, Tracker, Ctx, Handler); +return {}; I believe we should add another condition here: `VD->isL

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: I am afraid I might have found one more problem :( I believe that for `span

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: jkorous wrote: > I am afraid I might have found one more problem :( > I bel

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: jkorous wrote: > jkorous wrote: > > I am afraid I might have found one more

[PATCH] D143128: [-Wunsafe-buffer-usage][WIP] Fix-Its transforming `&DRE[any]` to `DRE.data() + any`

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:162 + InnerMatcher)), + unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage); + auto CastOperandMatcher = I am just wondering how does

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous accepted this revision. jkorous added a comment. LGTM Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139737/new/ https://reviews.llvm.org/D139737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. In D139737#4042093 , @NoQ wrote: > It sounds to me as if by landing the patch we'll temporarily make the > compiler emit incorrect fixes. I think we should avoid doing that. Is it > possible to wait until our first proof-of-conc

<    1   2   3   4   5   >