[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] 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] 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] 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] 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] 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 added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1196 const std::vector &Sanitizers, DiagnosticsEngine &Diags, SanitizerSet &S) { + bool Success = true; Can

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D95202: ADT: Use 'using' to inherit assign and append in SmallString

dexonsmith created this revision. dexonsmith added a reviewer: dblaikie. Herald added subscribers: ributzka, hiraditya. dexonsmith requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. Rather than reimplement, use a `using` declaration to br

[PATCH] D70701: Fix more VFS tests on Windows

dexonsmith added a comment. Thanks for the quick and detailed response! Your explanation of hybrid behaviour on Windows was especially helpful, as I didn't fully understand how that worked before. One thing I see, but wasn't obvious from your description, is that the mixed/hybrid separator beh

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

dexonsmith added a comment. I don't see a test for a case where the mapped directory already exists in the external FS, something like: # external fs /d1/f1 /d2/f2 # redirect fs from yaml (fallthrough: true) /d1 -> /d2 Unless I just missed it, can you add one? My intuition is we'd

[PATCH] D70701: Fix more VFS tests on Windows

dexonsmith added a comment. @rnk / @amccarth, I've been looking at the history of `makeAbsolute` being virtual, since it's a bit awkward that `RedirectingFileSystem` behaves differently from the others. I'm hoping you can give me a bit more context. I'm wondering about an alternative implementa

[PATCH] D95147: [clang][cli] Port visibility LangOptions 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/D95147/new/ https://reviews.llvm.org/D95147

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

dexonsmith added a comment. In D92191#2512960 , @saudi wrote: > Removed the support for "--" command lines, it was extracted to patch D95099 > . > Applied suggested fixes. Thanks! @Bigcheese, since you reviewed the origi

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

dexonsmith added a comment. Thanks for splitting this out! The looks good, just a few nits in line below. I also have a question about the test. Comment at: clang/lib/Tooling/Tooling.cpp:440 + // names. + auto ArgsEnd = Args.end(); + for (auto I = Args.begin(), E = Args.end

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

dexonsmith added a comment. It would be great if you could split the `--` support out into a separate patch to commit ahead of time, but I left a couple of comments since I was taking a look. Comment at: clang/lib/Tooling/Tooling.cpp:438-440 for (StringRef Arg : Args)

[PATCH] D94957: [clang][cli] Port more options to new parsing 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/D94957/new/ https://reviews.llvm.org/D94957

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

dexonsmith added a comment. In D94472#2507361 , @jansvoboda11 wrote: > Thanks for putting your time into the comment, @dexonsmith. > > I agree that adding a `-round-trip-args` flag to enable/disable > round-tripping is more sensible than hard-coding it v

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

dexonsmith added a reviewer: dexonsmith. dexonsmith added a comment. In D94844#2502725 , @nathawes wrote: > Yeah, certainly. Let me put that up separately and update this diff. Thanks! (Optionally, you can link them using the "edit related revisions" fea

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

dexonsmith added a comment. > This patch also refactors OverlayFileSystem's directory iterator > implementation (OverlayFSDirIterImpl) and VFSFromYamlDirIterImpl into a > single implementation, addressing a FIXME about their conceptual similarity. Can the `OverlayFSDirIterImpl` part be split in

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

dexonsmith added a comment. A concern I have is that the round-tripping might be too low level since it requires each group of options to opt-in somehow. Having something at the top-level that doesn't have to be touched again would be easier to validate / understand, and might better serve the

[PATCH] D94472: [clang][cli] Manually generate header search arguments

dexonsmith added a comment. It'd be much nicer to add the tests as part of the patch. I've added a comment to D94474 with a possible way to do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94472/new/ https:/

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

dexonsmith added a comment. I'm wondering if we can get this in incrementally without needing to list in code which options are correctly handled. Here's one way to do it: - Add a tool, `clang-cc1-args`, that shows the diff between round-tripping when given a set of args. (This could potentiall

[PATCH] D94474: [WIP][clang][cli] Test manual header search argument generation with round-trip

dexonsmith added inline comments. Comment at: clang/include/clang/Frontend/CompilerInvocation.h:199-203 void generateCC1CommandLine(llvm::SmallVectorImpl &Args, StringAllocator SA) const; + void generateCC1CommandLine(llvm::SmallVectorImpl &Args

[PATCH] D94488: [clang][cli] Port more CodeGenOptions to marshalling infrastructure

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM, with one nit. It'd be nice to add more detail to the commit message as well. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1146 - Opts.SSPBufferSize =

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing 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/D84673/new/ https://reviews.llvm.org/D84673

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

dexonsmith added a comment. Please also update the commit message to explain why this is safe (that `-fp-exception-behavior` fully encapsulates the semantics for `-cc1`) rather than just saying the options were ignored (which sounds like a bug). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

dexonsmith accepted this revision. dexonsmith added a comment. LGTM. I've just done a careful audit myself, and I'm now confident this patch is correct and that there is no latent bug -- that it's correct to ignore `-f*trapping-math` on the `-cc1` command-line since `-fp-exception-mode` will al

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. (Marking as "Request Changes" to drop this from my queue; feel free to reach out if the direction I suggested isn't working well...) Repository: rG LLVM Github Monorepo C

[PATCH] D93701: [clang][cli] NFC: Pass an ignoring DiagnosticsEngine instead of nullptr to ParseDiagnosticArgs

dexonsmith added a comment. With the above adjustment, this LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93701/new/ https://reviews.llvm.org/D93701 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D93701: [clang][cli] NFC: Pass an ignoring DiagnosticsEngine instead of nullptr to ParseDiagnosticArgs

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1400-1402 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, -DiagnosticsEngine *Diags, +DiagnosticsEngine

[PATCH] D93702: [clang][cli] NFC: Make marshalling macros reusable

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/D93702/new/ https://reviews.llvm.org/D93702

[PATCH] D94172: [clang][cli] NFC: Move parseSimpleArgs

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/D94172/new/ https://reviews.llvm.org/D94172

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

dexonsmith added inline comments. Comment at: clang/include/clang/Driver/Options.td:1193-1195 +defm caret_diagnostics : BoolFOption<"caret-diagnostics", + "DiagnosticOpts->ShowCarets", DefaultsToTrue, + ChangedBy, ResetBy>, IsDiag; There was one thing in the or

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

dexonsmith added a comment. In D78058#2480411 , @dexonsmith wrote: > In D78058#2471735 , @sammccall wrote: > >> @dexonsmith thanks for sharing! >> Some initial thoughts since abstracting outputs is something we're s

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

dexonsmith added a comment. In D78058#2471735 , @sammccall wrote: > @dexonsmith thanks for sharing! > Some initial thoughts since abstracting outputs is something we're starting > to care about too... Thanks for looking. > This doesn't appear to be an e

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

dexonsmith added inline comments. Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:102-107 + std::string getMacroName() const { +if (KeyPath.startswith("DiagnosticOpts.")) + return (Twine("DIAG_") + MarshallingInfo::MacroName).str(); + +return MarshallingInfo::M

[PATCH] D93702: [clang][cli] NFC: Make marshalling macros reusable

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3040 -#define OPTION_WITH_MARSHALLING( \ -PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \ -HELPTEXT, META

[PATCH] D93701: [clang][cli] NFC: Make Diags optional in normalizer

dexonsmith added a comment. In D93701#2477239 , @jansvoboda11 wrote: > This will make more sense after looking at D84673 > . In short: parsing of diagnostic options > can happen outside of `CompilerInvocation::createFrom

[PATCH] D93701: [clang][cli] NFC: Make Diags optional in normalizer

dexonsmith added a comment. In D93701#2479574 , @jansvoboda11 wrote: > In D93701#2477239 , @jansvoboda11 > wrote: > >> Another solution would be to create a "dummy" `DiagnosticsEngine` for the >> `ParseDiagnostic

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

dexonsmith added a comment. In D78058#2464262 , @dexonsmith wrote: > I'll reply here once I've posted the RFC and patch (as I said, I'm hoping > next week) so you can take a look. I don't quite have my patch and RFC ready (and I may not until I'm back fr

[PATCH] D93148: Basic: Add native support for stdin to SourceManager and FileManager

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3ee43adfb20d: Basic: Add native support for stdin to SourceManager and FileManager (authored by dexonsmith). Changed prior to commit: https://revi

[PATCH] D92531: Basic: Support named pipes natively in SourceManager

This revision was automatically updated to reflect the committed changes. Closed by commit rG245218bb3555: Basic: Support named pipes natively in SourceManager and FileManager (authored by dexonsmith). Changed prior to commit: https://reviews.llvm.org/D92531?vs=311004&id=313618#toc Repository:

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

dexonsmith added inline comments. Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:102-107 + std::string getMacroName() const { +if (KeyPath.startswith("DiagnosticOpts.")) + return (Twine("DIAG_") + MarshallingInfo::MacroName).str(); + +return MarshallingInfo::M

[PATCH] D93702: [clang][cli] NFC: Make marshalling macros reusable

dexonsmith added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3040 -#define OPTION_WITH_MARSHALLING( \ -PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \ -HELPTEXT, META

[PATCH] D93701: [clang][cli] NFC: Make Diags optional in normalizer

dexonsmith added a comment. > This is needed for a future patch, where we start using normalizers in > contexts where no Diags are available. Can you explain what those contexts are? I'm wondering if they can be changed to create a `DiagnosticsEngine` instead of having to account for it being m

[PATCH] D93679: [clang][cli] Port getAllArgumentValues to the marshalling infrastructure

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/D93679/new/ https://reviews.llvm.org/D93679

[PATCH] D93698: [clang][cli] Port a CommaJoined option to the marshalling infrastructure

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM, with one nit. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:333-337 +for (const std::string &Value : Values) { + if (&Value != &Values.front())

[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. This looks like an improvement, so LGTM, but I have a couple of comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:160-161 -namespace { -template stru

[PATCH] D93631: [clang][cli] Implement `getAllArgValues` marshalling

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:338 +const std::vector &Values) { + for (const std::string& Value : V

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

dexonsmith added subscribers: erik.pilkington, arphaman, Bigcheese. dexonsmith added a comment. I think this is important; thanks for working on it! I'm sorry I missed it before... coincidentally, I'm working in a similar problem space right now (likely I'll post a patch with an RFC next week) b

<    1   2   3   4   5   6   7   8   9   10   >