[PATCH] D95221: [clang][cli] NFC: Move prefix to the front of BoolOption

dexonsmith accepted this revision. dexonsmith 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/D95221/new/ https://reviews.llvm.org/D95221

[PATCH] D95225: [clang][cli] NFC: Pass CC1Option explicitly to BoolOption

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM; this is much better! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95225/new/ https://reviews.llvm.org/D95225 ___

[PATCH] D70701: Fix more VFS tests on Windows

dexonsmith added a comment. In D70701#2515934 , @amccarth wrote: > BTW, I hope I didn't come across as overly negative in my previous > response. No, not at all! > [...] On Windows, a process can have multiple > current directories: one for each drive.

[PATCH] D95202: ADT: Use 'using' to inherit assign and append in SmallString

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGba5628f2c2a9: ADT: Use 'using' to inherit assign and append in SmallString (authored by dexonsmith). Changed prior to commit: https://reviews.llvm

[PATCH] D95202: ADT: Use 'using' to inherit assign and append in SmallString

dexonsmith marked 2 inline comments as done. dexonsmith added a comment. Thanks for the review! Pushed with suggestions applied in ba5628f2c2a9de049b80b3e276f7e05f481c49e7 . Comment at: llvm/lib/Support/File

[PATCH] D95279: Support: Remove duplicated code in {File,clang::ModulesDependency}Collector, NFC

dexonsmith created this revision. dexonsmith added reviewers: dblaikie, JDevlieghere. Herald added subscribers: ributzka, hiraditya. dexonsmith requested review of this revision. Herald added projects: clang, LLVM. Refactor the duplicated canonicalize-path logic in `FileCollector` and `ModulesDepe

[PATCH] D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options

dexonsmith added a comment. Thanks for talking through this with me! The direction sounds good. In D94472#2519838 , @jansvoboda11 wrote: > I think the driver command line options could be useful even after we're > finished You mean the `-cc1` options u

[PATCH] D95348: [clang][cli] Port OpenMP-related LangOpts to marshalling system

dexonsmith accepted this revision. dexonsmith 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/D95348/new/ https://reviews.llvm.org/D95348

[PATCH] D95344: [clang][cli] Accept strings instead of options in ImpliedByAnyOf

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. > This somewhat weakens the guarantees that we're referring to an existing > (option) record, but we can still use the option.KeyPath syntax to simulate > this. I agree that us

[PATCH] D95345: [clang][cli] Port GPU-related language options to marshalling system

dexonsmith accepted this revision. dexonsmith 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/D95345/new/ https://reviews.llvm.org/D95345

[PATCH] D95346: [clang][cli] Port LangOpts to marshalling system, pt.1

dexonsmith accepted this revision. dexonsmith 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/D95346/new/ https://reviews.llvm.org/D95346

[PATCH] D95340: [clang][cli] NFC: Simplify BoolOption API

dexonsmith accepted this revision. dexonsmith 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/D95340/new/ https://reviews.llvm.org/D95340

[PATCH] D95342: [clang][cli] Store LangStandard::Kind in LangOptions

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/include/clang/Basic/LangOptions.h:18 #include "clang/Basic/CommentOptions.h" +#include "clang/Basic/LangStandard.h" #include "clang/Basic/LL

[PATCH] D95347: [clang][cli] Port LangOpts to marshalling system, pt. 2

dexonsmith accepted this revision. dexonsmith 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/D95347/new/ https://reviews.llvm.org/D95347

[PATCH] D95343: [clang][cli] Port GNU language options to marshalling system

dexonsmith accepted this revision. dexonsmith 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/D95343/new/ https://reviews.llvm.org/D95343

[PATCH] D94802: [clang][cli] Parse HeaderSearch options separately

dexonsmith accepted this revision. dexonsmith 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/D94802/new/ https://reviews.llvm.org/D94802

[PATCH] D94803: [clang][cli] Generate HeaderSearch options separately

dexonsmith accepted this revision. dexonsmith 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/D94803/new/ https://reviews.llvm.org/D94803

[PATCH] D95279: Support: Remove duplicated code in {File,clang::ModulesDependency}Collector, NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG080952a9447a: Support: Remove duplicated code in {File,clang::ModulesDependency}Collector, NFC (authored by dexonsmith). Changed prior to commit:

[PATCH] D92983: SourceManager: Migrate to FileEntryRef in getOrCreateContentCache, NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8d67b9e2461d: SourceManager: Migrate to FileEntryRef in getOrCreateContentCache, NFC (authored by dexonsmith). Changed prior to commit: https://re

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4d02fbe418d: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble… (authored by dexonsmith). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

dexonsmith added a comment. Thanks for the reviews; pushed f4d02fbe418db55375b78b8f57e47126e4642fb6 . Comment at: clang/include/clang/Frontend/PrecompiledPreamble.h:108 bool CanReuse(const CompilerInvocati

[PATCH] D93249: Frontend: Fix memory leak in CompilerInstance::setVerboseOutputStream

dexonsmith added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93249/new/ https://reviews.llvm.org/D93249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D93248: Frontend: Fix layering between create{,Default}OutputFile, NFC

dexonsmith added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93248/new/ https://reviews.llvm.org/D93248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D93260: Frontend: Simplify handling of non-seeking streams in CompilerInstance, NFC

dexonsmith added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93260/new/ https://reviews.llvm.org/D93260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options

dexonsmith added a comment. In D94472#2523053 , @Bigcheese wrote: > In D94472#2519838 , @jansvoboda11 > wrote: > >> In D94472#2508018 , @dexonsmith >> wrote: >> >>> `strict

[PATCH] D93249: Frontend: Fix memory leak in CompilerInstance::setVerboseOutputStream

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8afabff6b11c: Frontend: Fix memory leak in CompilerInstance::setVerboseOutputStream (authored by dexonsmith). Repository: rG LLVM Github Monorepo

[PATCH] D93260: Frontend: Simplify handling of non-seeking streams in CompilerInstance, NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f721476d10c: Frontend: Simplify handling of non-seeking streams in CompilerInstance, NFC (authored by dexonsmith). Repository: rG LLVM Github Mon

[PATCH] D93248: Frontend: Fix layering between create{,Default}OutputFile, NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGad7aaa475e5e: Frontend: Fix layering between create{,Default}OutputFile, NFC (authored by dexonsmith). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

dexonsmith created this revision. dexonsmith added a reviewer: erik.pilkington. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Call `FixupRelativePath` when opening output files to ensure that `-working-directory` is used when chec

[PATCH] D95502: WIP: Frontend: Adopt llvm::vfs::OutputManager in CompilerInstance

dexonsmith created this revision. dexonsmith added reviewers: sammccall, erik.pilkington, marcrasi. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. [This is supporting an RFC I'll post in a bit; marked WIP for now] Adopt the new ll

[PATCH] D78058: option to write files to memory instead of disk

dexonsmith added a comment. In D78058#2481050 , @dexonsmith wrote: > In D78058#2480411 , @dexonsmith > wrote: > >> In D78058#2471735 , @sammccall >> wrote: >> >>> @dexonsmi

[PATCH] D95502: WIP: Frontend: Adopt llvm::vfs::OutputManager in CompilerInstance

dexonsmith added a comment. RFC is up at https://lists.llvm.org/pipermail/cfe-dev/2021-January/067576.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95502/new/ https://reviews.llvm.org/D95502 ___ cfe

[PATCH] D95514: [clang][cli] Teach CompilerInvocation to allocate strings on its own

dexonsmith added a comment. Can you include the use in the patch? I’m not sure having the AllocateString API totally makes sense, vs moving in a string pool that was independently used during generation, but maybe in context? I also wonder what command-line option is forcing this. Ideally we co

[PATCH] D95514: [clang][cli] Teach CompilerInvocation to allocate strings on its own

dexonsmith added a comment. In D95514#2525517 , @jansvoboda11 wrote: > In D95514#2525255 , @dexonsmith > wrote: > >> Can you include the use in the patch? >> I also wonder what command-line option is forcing this.

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

dexonsmith added inline comments. Comment at: clang/include/clang/Driver/Options.td:593-594 +def round_trip_args : Flag<["-"], "round-trip-args">, Flags<[CC1Option, NoDriverOption]>, + HelpText<"Performs 'parse-generate-parse' round-trip of command line arguments.">; +def ro

[PATCH] D95532: [clang][cli] Use variadic macros for parsing/generating

dexonsmith accepted this revision. dexonsmith 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/D95532/new/ https://reviews.llvm.org/D95532

[PATCH] D94844: [VFS] Add support to RedirectingFileSystem for mapping a virtual directory to one in the external FS.

dexonsmith added a comment. I haven't looked at the code in detail (deferring to @JDevlieghere), but the new tests look great. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94844/new/ https://reviews.llvm.org/D94844 ___ cfe-commits m

[PATCH] D95561: [Clang] Introduce Swift async calling convention.

dexonsmith added a comment. In D95561#2526417 , @varungandhi-apple wrote: > I don't get how you stack your change on top of someone else's change (the > diff seems to have been created with only one commit), but this depends on > https://reviews.llvm.or

[PATCH] D95581: Frontend: Refactor compileModuleAndReadAST, NFC

dexonsmith created this revision. dexonsmith added reviewers: Bigcheese, jansvoboda11. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Herald added a project: clang. This renames `compileModuleAndReadAST`, adding a `BehindLock` suffix, and refactors it to signifi

[PATCH] D95583: Frontend: Add -f{, no-}implicit-modules-uses-lock and -Rmodule-lock

dexonsmith created this revision. dexonsmith added reviewers: Bigcheese, jansvoboda11. Herald added subscribers: dang, ributzka. dexonsmith requested review of this revision. Herald added a project: clang. Add -cc1 flags `-fmodules-uses-lock` and `-fno-modules-uses-lock` to allow the lock manager

[PATCH] D95628: WIP: clang-scan-deps: Skip the disk when scanning modules dependencies

dexonsmith created this revision. dexonsmith added reviewers: Bigcheese, arphaman, sammccall. Herald added subscribers: ributzka, tschuett. dexonsmith requested review of this revision. Herald added a project: clang. Modules dependendency scanning builds lightweight modules (based on minimized pre

[PATCH] D95514: [clang][cli] Teach CompilerInvocation to allocate strings on its own

dexonsmith added a comment. In D95514#2528407 , @Bigcheese wrote: > In D95514#2528324 , @jansvoboda11 > wrote: > >> I've looked through all option classes in `CompilerInvocation`, and >> `AnalyzerOptions::Config`

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. This is getting close! A few more comments inline. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:584-589 +static bool Equal(const ArgStringList &A,

[PATCH] D95366: [clang][cli] Generate and round-trip preprocessor options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3093 + +static bool ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,

[PATCH] D95629: [clang][analyzer] Own string keys in AnalyzerOptions::ConfigTable

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. > This patch replaces StringMap<...> with std::map in > AnalyzerOptions::ConfigTable. It seems to be the only non-owning reference to > command line arguments in the whole Co

[PATCH] D95369: [clang][cli] Generate and round-trip analyzer options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:822 + static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:684-685 + if (!GeneratedStringsEqual) +llvm::report_fatal_error("Mismatch between arguments generated during " + "round-trip"); + jansvoboda11

[PATCH] D95583: Frontend: Add -f{, no-}implicit-modules-uses-lock and -Rmodule-lock

dexonsmith added inline comments. Comment at: clang/test/Modules/implicit-modules-use-lock.m:21 + +// CHECK-NO-LOCKS-NOT: remark: +// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' [-Rmodule-lock] jansvoboda11 wrote: > Where is the empty remark co

[PATCH] D95369: [clang][cli] Generate and round-trip analyzer options

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:822 + static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args, DiagnosticsEngine &Diags) { jansvoboda11 wrote: > dexonsmith wrote: >

[PATCH] D95792: [clang][cli] Report result of ParseLangArgs

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3096 + + return Success; } Would this avoid the changes above? ``` return Success && !Diags.hasErrorOccurred(); ``` ? Alternatively, might it be cleaner to shove the `Succe

[PATCH] D95793: [clang][cli] Generate and round-trip language options

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1196 const std::vector &Sanitizers, DiagnosticsEngine &Diags, SanitizerSet &S) { + bool Success = true; Can

[PATCH] D95879: [clang][index] Mark file as C++ in parse-all-comments test

dexonsmith accepted this revision. dexonsmith 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/D95879/new/ https://reviews.llvm.org/D95879 _

[PATCH] D95793: [clang][cli] Generate and round-trip language options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM, since I don't think the parallel error paths are too bad in this specific patch, but see my longer comment inline. Comment at: clang/lib/Frontend/CompilerInvoc

[PATCH] D95792: [clang][cli] Report result of ParseLangArgs

dexonsmith accepted this revision. dexonsmith 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/D95792/new/ https://reviews.llvm.org/D95792

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. In D94472#2539685 , @jansvoboda11 wrote: > I've changed this patch so that the errors and debugging output goes to > `DiagnosticsEngine`. > >

[PATCH] D95099: [clang-scan-deps] : Support -- in clang command lines.

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95099/new/ https://reviews.llvm.org/D95099 ___ cfe-commits mailing list cfe-co

[PATCH] D96036: [clang][codegen] Remember string used to create llvm::Regex for optimization remarks

dexonsmith accepted this revision. dexonsmith added a comment. LGTM! I also have a couple of optional suggestions to consider. Comment at: clang/include/clang/Basic/CodeGenOptions.h:286 + +bool isEnabled() const { return Regex != nullptr; } + This could als

[PATCH] D96156: [clang][cli] Generate and round-trip PreprocessorOutput options

dexonsmith accepted this revision. dexonsmith 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/D96156/new/ https://reviews.llvm.org/D96156

[PATCH] D109898: [clang][NFC] refactor GlobalMethodPool to encapsulate its map

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. Thanks for splitting this out -- left some comments inline, LGTM after that. Comment at: clang/include/clang/Sema/Sema.h:1422-1427 /// Method Pool - allows efficie

[PATCH] D109632: [clang] de-duplicate methods from AST files

dexonsmith added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1434-1436 +bool addMethod(ObjCMethodDecl *Method) { + return AddedMethods.insert(Method).second; +} Hmm, I was imagining that the set would be more encapsulated than this,

[PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

dexonsmith added a comment. In D109345#3008426 , @dblaikie wrote: > Thanks for the suggestions/details, @dexonsmith - I've posted to llvm-dev > here: https://groups.google.com/g/llvm-dev/c/m9UVRhzJvh4/m/qdd_SyPuCQAJ and > will wait for some follow-up (

[PATCH] D109632: [clang] de-duplicate methods from AST files

dexonsmith added a comment. In D109632#3007615 , @vsapsai wrote: > I don't remember for sure but I don't think there is a consistent policy > about a module storing transitive data or only data it owns. I suspect we > might be using both approaches and

[PATCH] D109632: [clang] de-duplicate methods from AST files

dexonsmith added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1434-1436 +bool addMethod(ObjCMethodDecl *Method) { + return AddedMethods.insert(Method).second; +} rmaz wrote: > rmaz wrote: > > dexonsmith wrote: > > > Hmm, I was imagini

[PATCH] D110160: [clang][tooling] NFC: Refactor command-line diagnostic tests

dexonsmith added inline comments. Comment at: clang/unittests/Tooling/ToolingTest.cpp:238-245 Args.push_back("tool-executable"); - Args.push_back("-target"); - // Invalid argument that by default results in an error diagnostic: - Args.push_back("i386-apple-ios14.0-simulator

[PATCH] D109632: [clang] de-duplicate methods from AST files

dexonsmith added a comment. In D109632#3013523 , @vsapsai wrote: > 2. Serialize only methods owned by the current module (and change > `ReadMethodPoolVisitor` appropriately). Would that require visiting all in-memory modules every time there's a global

[PATCH] D110068: [Clang][AST] Resolve FIXME: Remove ObjCObjectPointer from isSpecifierType

dexonsmith added a comment. I'm a bit hesitant to trust lack of tests as proof that old FIXMEs can be resolved safely. Can you add some background information about why this was originally a specifier, and why it's safe to fix it now? (If you don't know, I suggest looking through git-blame to f

[PATCH] D110160: [clang][tooling] NFC: Refactor command-line diagnostic tests

dexonsmith added a comment. Stepping back, maybe it'd be better to use a more generic option that is unlikely to have special behaviour in any target (or downstream). Best one I could find is triggering `err_drv_unknown_language` with `-x this-is-an-unknown-language`. WDYT?

[PATCH] D110160: [clang][tooling] NFC: Refactor command-line diagnostic tests

dexonsmith accepted this revision. dexonsmith 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/D110160/new/ https://reviews.llvm.org/D110160 __

[PATCH] D96280: [WIP][clang][cli] Round-trip the whole CompilerInvocation

dexonsmith added a comment. Mostly looks great; happy to see this come full circle and remove the round-tripping boilerplate. Just a couple of questions inline below. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1940-1941 case EDK_ProfileList: - GenerateArg(

[PATCH] D96278: [clang][cli] Extract FileSystem and Migrator options parsing/generation

dexonsmith accepted this revision. dexonsmith 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/D96278/new/ https://reviews.llvm.org/D96278

[PATCH] D96274: [clang][cli] Generate and round-trip Diagnostic options

dexonsmith requested changes to this revision. dexonsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:745 + +A->render(Args, Rendered); } It's not obvious why this renders the

[PATCH] D96149: [clang][cli] Store InputKind in FrontendOptions

dexonsmith accepted this revision. dexonsmith 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/D96149/new/ https://reviews.llvm.org/D96149

[PATCH] D96273: [clang][cli] Generate and round-trip DependencyOutput options

dexonsmith accepted this revision. dexonsmith 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/D96273/new/ https://reviews.llvm.org/D96273

[PATCH] D96158: [clang][cli] Generate and round-trip Target options

dexonsmith accepted this revision. dexonsmith 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/D96158/new/ https://reviews.llvm.org/D96158

[PATCH] D96155: [clang][cli] Generate and round-trip Frontend options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM, although the description seems incorrect: > This patch implements generation of remaining preprocessor options and tests > it by performing parse-generate-parse round trip. In f

[PATCH] D96056: [clang][cli] Generate and round-trip CodeGen options

dexonsmith accepted this revision. dexonsmith 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/D96056/new/ https://reviews.llvm.org/D96056

[PATCH] D96269: [clang][cli] Look up program action argument in a table

dexonsmith accepted this revision. dexonsmith 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/D96269/new/ https://reviews.llvm.org/D96269

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. This is similar enough to the original patch I'm comfortable signing off, even though @Bigcheese hasn't had a chance to take a look. CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D95055: [clang] Don't look into for C++ headers if they are found alongside the toolchain

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Sorry for missing this earlier! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95055/new/ https://reviews.llvm.org/D95055

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

dexonsmith added a comment. In D92808#2545873 , @ahatanak wrote: > I ended up reverting the changes I made to `llvm/lib/IR/AutoUpgrade.cpp` as > the file was including `llvm/Analysis/ObjCARCUtil.h`, which was violating > layering. Are you planning to la

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

dexonsmith added a comment. I have a couple of suggestions below to add to the mix (but I don't feel strongly about either of them, just adding them for consideration). In D92808#2552100 , @ahatanak wrote: > define void @test() { > %r = call i8* @f

[PATCH] D96274: [clang][cli] Generate and round-trip Diagnostic options

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM! In D96274#2551206 , @probinson wrote: > I don't claim to be very familiar with this area but "round-trip" and "test" > would make me thi

[PATCH] D96323: [clang][cli] NFC: Remove intermediate command line parsing functions

dexonsmith accepted this revision. dexonsmith 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/D96323/new/ https://reviews.llvm.org/D96323

[PATCH] D96280: [clang][cli] Round-trip the whole CompilerInvocation

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM! One more more comment inline. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4534-4547 + InputKind DashX = FrontendOpts.DashX; + if (DashX.getFormat()

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

dexonsmith added a comment. In D92808#2552354 , @rjmccall wrote: > The ultimate code-generation consequences of this feature aren't very > generic, so we shouldn't let ourselves get caught up designing an extremely > general feature that ultimately eithe

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

dexonsmith added a comment. In D92808#2555868 , @ahatanak wrote: > For example, if SCCP just does a normal RAUW on the following call, which is > taken from the example I posted, > > %r = call i8* @foo() [ "clang.arc.rv"(i64 1, i8* %r) ] > > will become

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

dexonsmith added a comment. In D92808#2556542 , @fhahn wrote: > @dexonsmith @rjmccall What do you think? Would you be happy with iterating > on the suggestions in tree? I don't feel strongly either way; happy for it to land as-is (bugs fixed) and itera

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

dexonsmith updated this revision to Diff 323422. dexonsmith added a comment. ping / rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95497/new/ https://reviews.llvm.org/D95497 Files: clang/lib/Frontend/CompilerInstance.cpp clang/test/Frontend/output-paths.c Index: clang/test/

[PATCH] D95502: WIP: Frontend: Adopt llvm::vfs::OutputManager in CompilerInstance

dexonsmith updated this revision to Diff 323435. dexonsmith added a comment. Herald added a subscriber: jfb. Rebase on top of newer https://reviews.llvm.org/D95501. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95502/new/ https://reviews.llvm.org/D95502 Files: clang/include/clang/Fro

[PATCH] D96705: [clang][cli] Add explicit round-trip test

dexonsmith accepted this revision. dexonsmith 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/D96705/new/ https://reviews.llvm.org/D96705

[PATCH] D96848: [clang][cli] Don't emit manufactured warnings

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2347-2353 +// This warning was manufactured, don't put it on the command line. +if (Warning == "no-stdlibcxx-not-found" && T.isOSDarwin() && +DashX.isPreprocessed()) + con

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

dexonsmith added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:306-307 + // The kind of optimization remark argument on the command line. + RemarkKind OptimizationRemarkKind; /// Regular expression to select optimizations for which we should enabl

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

dexonsmith added a comment. In D96816#2572552 , @ahatanak wrote: > I still don't understand why the test was failing, but did we have to bump > the version because a new LangOpt (`EncodeCXXClassTemplateSpec`) was added? Here are the RUN lines: // RUN:

[PATCH] D96974: [clang][patch] Inclusive language, modify filename SanitizerBlacklist.h to NoSanitizeList.h

dexonsmith added inline comments. Comment at: clang/lib/Basic/SanitizerBlacklist.cpp:15 #include "clang/Basic/FileManager.h" +#include "clang/Basic/NoSanitizeList.h" #include "clang/Basic/SanitizerSpecialCaseList.h" vitalybuka wrote: > Own header should go firs

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

dexonsmith added a comment. In D96847#2574408 , @jansvoboda11 wrote: > That's a bit nicer. > > Not sure if `RemarkPattern` is a good name now, as it may represent an > optimization remark that doesn't have any pattern associated with it. > How about call

[PATCH] D97042: [clang][cli] Stop creating '-Wno-stdlibcxx-not-found' in cc1

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

[PATCH] D97042: [clang][cli] Stop creating '-Wno-stdlibcxx-not-found' in cc1

dexonsmith added a comment. In D97042#2576372 , @dexonsmith wrote: > Agreed; LGTM. To be more explicit, here's how I confirmed this is dead code: `warn_drv_libstdcxx_not_found` is the only warning in `DiagGroup<"stdlibcxx-not-found">` and it's only used

[PATCH] D90497: Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, and DirectoryName, NFC

dexonsmith reopened this revision. dexonsmith added a comment. This revision is now accepted and ready to land. This has unexpectedly caused a behaviour change (perhaps the same root cause as https://reviews.llvm.org/D92975#2453482, but the investigation is ongoing). I'll be reverting later toda

[PATCH] D97288: Added `Follow` parameter to llvm::vfs::FileSystem::status()

dexonsmith added a comment. If we do add this, I'd suggest a separate function rather than modifying the current one. I also think implementing it should be optional. Something like: virtual ErrorOr status(const Twine &Path) = 0; virtual ErrorOr status(const Twine &Path, bool FollowSymlinks)

[PATCH] D97375: [clang][cli] Add MarshallingInfoEnum multiclass

dexonsmith accepted this revision. dexonsmith 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/D97375/new/ https://reviews.llvm.org/D97375

<    4   5   6   7   8   9   10   11   12   13   >