Page MenuHomePhabricator

hintonda (Don Hinton)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 18 2013, 5:29 PM (382 w, 6 d)

Recent Activity

Apr 26 2020

hintonda abandoned D17407: [Sema] PR25755 Handle out of order designated initializers .
Apr 26 2020, 9:33 AM · Restricted Project
hintonda abandoned D70263: [Error] Add source location to Errors.
Apr 26 2020, 9:33 AM · Restricted Project
hintonda abandoned D70259: [Error] Add source location to cantFail.
Apr 26 2020, 9:33 AM · Restricted Project

Apr 23 2020

hintonda abandoned D16971: [Sema] PR26077 Fix assert when partial specialization is missing required parameters.
Apr 23 2020, 11:53 AM
hintonda abandoned D62353: [cmake] Add new LLVM_CACHE_VARIABLES variable to contains all variables passed to cmake on the commandline or in cache files..
Apr 23 2020, 11:52 AM · Restricted Project
hintonda abandoned D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.
Apr 23 2020, 11:52 AM · Restricted Project
hintonda abandoned D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
Apr 23 2020, 11:52 AM · Restricted Project
hintonda abandoned D71438: [CommandLine] Move categories out of Option and into parser [NFC].
Apr 23 2020, 11:52 AM · Restricted Project
hintonda abandoned D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].
Apr 23 2020, 11:52 AM · Restricted Project

Apr 6 2020

hintonda added inline comments to D62445: [test] Fix plugin tests.
Apr 6 2020, 7:38 PM · Restricted Project, Restricted Project

Jan 15 2020

hintonda abandoned D72406: [Orc][LLJIT] Add optimizer support to LLJIT.

@lhames took care of this in e9e26c01cd865da678b1af6ba5f417c713956a66 yesterday.

Jan 15 2020, 9:44 AM · Restricted Project

Jan 8 2020

hintonda added a comment to D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].

post holiday ping...

Jan 8 2020, 12:41 PM · Restricted Project
hintonda added a reviewer for D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC]: alexr.
Jan 8 2020, 12:41 PM · Restricted Project
hintonda added a reviewer for D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC]: alexr.
Jan 8 2020, 12:41 PM · Restricted Project
hintonda added a comment to D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].

post holiday ping...

Jan 8 2020, 12:41 PM · Restricted Project
hintonda created D72406: [Orc][LLJIT] Add optimizer support to LLJIT.
Jan 8 2020, 11:18 AM · Restricted Project

Jan 3 2020

hintonda added a comment to D70600: [Error] Add stack traces for llvm::Error invariant violations..

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way.

I don't follow this - are you saying if heap allocation is OK, locking a mutex must be too, as it's cheaper?
Locking/unlocking a shared global mutex can certainly be more expensive than malloc if there's a lot of contention. malloc implementations typically go to some lengths to avoid a global lock for this reason.

Example workload: clangd typically runs ~30 threads for background indexing, each thread frequently calls functions returning Expected<T> and recovers from errors (Error is documented as being for recoverable errors). It'd be a big surprise if this sort of use of Error led to contention/serialization on these worker threads.

Note this feature's only intended to be opt-in for debugging purposes, so for production use cases it shouldn't be a huge deal (though could be cheaper/free if it was a build-time setting rather than something you could opt into/out of at runtime - perhaps that's the point you're making? Though we do check various globals (cl::opts, etc) in a fair number of codepaths, I think, such that adding one doesn't seem likely to be super problematic?). (as much as I agree with you that I'd rather not have a side table)

Jan 3 2020, 4:27 PM · Restricted Project

Dec 17 2019

hintonda added a comment to D71438: [CommandLine] Move categories out of Option and into parser [NFC].

I recall that you have a patch that touched this area, which was committed and reverted for several times. Is this patch a similar one? Can you find pointers to the old one?

Dec 17 2019, 4:39 PM · Restricted Project

Dec 12 2019

hintonda retitled D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC] from [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list NFC to [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].
Dec 12 2019, 2:58 PM · Restricted Project
hintonda retitled D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC] from [Commandline] Move general LLVMSupport options into Commandline.cpp to [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
Dec 12 2019, 2:57 PM · Restricted Project
hintonda retitled D71438: [CommandLine] Move categories out of Option and into parser [NFC] from [CommandLine] Move categories out of Option and into parser [NFC} to [CommandLine] Move categories out of Option and into parser [NFC].
Dec 12 2019, 2:57 PM · Restricted Project
hintonda created D71438: [CommandLine] Move categories out of Option and into parser [NFC].
Dec 12 2019, 2:48 PM · Restricted Project

Dec 11 2019

hintonda updated the summary of D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
Dec 11 2019, 9:23 PM · Restricted Project
hintonda retitled D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC] from [Commandline] Move Debug options from Debug.cpp to Commandline.cpp to [Commandline] Move general LLVMSupport options into Commandline.cpp.
Dec 11 2019, 9:14 PM · Restricted Project
hintonda updated the diff for D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
  • Also move over DisableSymbolication option from Signals.cpp, and put all variables in llvm namespace.
Dec 11 2019, 5:31 PM · Restricted Project
hintonda added inline comments to D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
Dec 11 2019, 3:33 PM · Restricted Project

Dec 10 2019

hintonda added a comment to D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].

This patch moves Debug options into CommandLine.cpp so they are only constructed if parseCommandLineOptions is actually called.

This would be the case if the options were static to the parseCommandLineOptions, but I fail ti understand why current change gives this property. Can you explain me why?

When you statically link in LLVMSupport, you only load what you reference. In the case of Debug options, they are referenced all over, so that TU gets loaded. If it also included static Options, which register themselves with the global parser in CommandLine.cpp, that TU gets loaded as well. By moving the Debug options over to CommandLine.cpp, those statics won't be loaded unless the TU is loaded, which will only happened if the user calls function in CommandLine.cpp.

We have a case where we use functionality in LLVMSupport, but don't call parseCommandLineOptions. We can still access the DebugFlag, etc, but don't actually need the option since it is only used by the parser -- those options use cl::location, so the global already exists. Hope that makes sense.

Dec 10 2019, 2:46 PM · Restricted Project
hintonda added a comment to D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].

This patch moves Debug options into CommandLine.cpp so they are only constructed if parseCommandLineOptions is actually called.

This would be the case if the options were static to the parseCommandLineOptions, but I fail ti understand why current change gives this property. Can you explain me why?

Dec 10 2019, 2:00 PM · Restricted Project

Dec 9 2019

hintonda created D71228: [Commandline] Move general LLVMSupport options into Commandline.cpp [NFC].
Dec 9 2019, 2:08 PM · Restricted Project

Dec 7 2019

hintonda updated the diff for D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].
  • Remove unneeded cl::ValueRequired, since it is the default
Dec 7 2019, 1:03 PM · Restricted Project
hintonda created D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].
Dec 7 2019, 12:43 PM · Restricted Project

Dec 6 2019

hintonda committed rG6555995a6d45: [CommandLine] Add callbacks to Options (authored by hintonda).
[CommandLine] Add callbacks to Options
Dec 6 2019, 3:30 PM
hintonda closed D70620: [CommandLine] Add callbacks to Options.
Dec 6 2019, 3:30 PM · Restricted Project
hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Address comments
Dec 6 2019, 10:31 AM · Restricted Project

Dec 5 2019

hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Rename traits and group asserts for readability. Add example to help.
Dec 5 2019, 8:09 AM · Restricted Project

Dec 4 2019

hintonda added inline comments to D70620: [CommandLine] Add callbacks to Options.
Dec 4 2019, 10:45 PM · Restricted Project
hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Add static_asserts for arg_type
Dec 4 2019, 8:53 PM · Restricted Project
hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Extract arg type from callback so caller won't need to provide it.
Dec 4 2019, 7:29 PM · Restricted Project

Dec 3 2019

hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Address comments.
Dec 3 2019, 2:17 PM · Restricted Project

Dec 2 2019

hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Remove void* and templatize callback struct.
Dec 2 2019, 6:12 PM · Restricted Project

Nov 27 2019

hintonda added a comment to D70628: [Support] Enable file + line info in LLVM stack traces on Darwin..

Just a standard llvm Debug build. Is there an cmake option I'm missing?

I'm not sure whether there is a cmake option to generate dSYMs yet -- you'll have to run dsymutil on whatever tool you're testing with.

I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

I suppose that might be useful for individuals, but I don't think it would be a good idea for the bots, which is my primary target.

Nov 27 2019, 2:17 PM · Restricted Project
hintonda added a comment to D70628: [Support] Enable file + line info in LLVM stack traces on Darwin..

Just a standard llvm Debug build. Is there an cmake option I'm missing?

I'm not sure whether there is a cmake option to generate dSYMs yet -- you'll have to run dsymutil on whatever tool you're testing with.

I think the best long term solution to this problem is to teach llvm-symbolize how to cope with debug info in objects, rather than requiring a dSYM. An intermediate step (possibly generically useful) would be to add an option to llvm-symbolize to run llvm-dsymutil if there's no dSYM available.

Nov 27 2019, 11:23 AM · Restricted Project

Nov 25 2019

hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Remove unnecessary casts.
Nov 25 2019, 7:15 AM · Restricted Project

Nov 24 2019

hintonda added a comment to D70620: [CommandLine] Add callbacks to Options.

I'd just like to acknowledge a few issues with this patch, and request feedback:

Nov 24 2019, 10:55 AM · Restricted Project
hintonda added a comment to D70628: [Support] Enable file + line info in LLVM stack traces on Darwin..

This is awesome, thanks for working on it!

I pulled down your patch and built it, but am not getting expected results. Did I miss something?

Did you generate a dSYM for the binary with dsymutil? Unfortunately llvm-symbolize doesn't seem to know how to symbolize directly from objects containing debug info, but I'm hoping we can eventually teach it to do that.

Nov 24 2019, 9:14 AM · Restricted Project
hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Fixed typos.
Nov 24 2019, 8:19 AM · Restricted Project
hintonda added inline comments to D70620: [CommandLine] Add callbacks to Options.
Nov 24 2019, 8:19 AM · Restricted Project
hintonda retitled D70620: [CommandLine] Add callbacks to Options from [CommandLine] Add cl::implies option attribute to [CommandLine] Add callbacks to Options.
Nov 24 2019, 8:14 AM · Restricted Project
hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Add support for callbacks for Options, and remove cl::implied, since they are now OBE.
Nov 24 2019, 8:10 AM · Restricted Project

Nov 23 2019

hintonda added a comment to D70620: [CommandLine] Add callbacks to Options.

This looks nice. I think it's missing an entry somewhere in the user doc (so that people can start using it!).

Nov 23 2019, 11:42 AM · Restricted Project
hintonda added a comment to D70628: [Support] Enable file + line info in LLVM stack traces on Darwin..

This is awesome, thanks for working on it!

Nov 23 2019, 11:33 AM · Restricted Project

Nov 22 2019

hintonda updated the diff for D70620: [CommandLine] Add callbacks to Options.
  • Fix test case order.
Nov 22 2019, 3:01 PM · Restricted Project
hintonda created D70620: [CommandLine] Add callbacks to Options.
Nov 22 2019, 3:01 PM · Restricted Project

Nov 21 2019

hintonda updated the diff for D70263: [Error] Add source location to Errors.

Switch to builder.

Nov 21 2019, 10:03 PM · Restricted Project
hintonda retitled D70263: [Error] Add source location to Errors from [Error] Add source location macro to [Error] Add source location to Errors.
Nov 21 2019, 10:03 PM · Restricted Project

Nov 19 2019

hintonda added a comment to D70259: [Error] Add source location to cantFail.

@lhames, I think I've applied all your verbal changes, but please let me
know if I missed anything.

Nov 19 2019, 2:06 PM · Restricted Project
hintonda updated the summary of D70259: [Error] Add source location to cantFail.
Nov 19 2019, 1:56 PM · Restricted Project
hintonda updated the diff for D70259: [Error] Add source location to cantFail.

Append "_annotated" suffix to detail functions.
Add "llvm_" prefix to macros.
Revert changes to all non-llvm project files.
Update calls in llvm to use new macro.

Nov 19 2019, 1:47 PM · Restricted Project
hintonda updated the diff for D70259: [Error] Add source location to cantFail.
  • Add back original llvm::cantFail signatures so they'll still be
Nov 19 2019, 12:43 PM · Restricted Project
hintonda updated the diff for D70259: [Error] Add source location to cantFail.
  • Replace macro magic with matching 3-parameter template functions.
  • Refactor cantFail move into detail namespace. Change macro name to
Nov 19 2019, 12:33 PM · Restricted Project

Nov 15 2019

hintonda added a comment to D70263: [Error] Add source location to Errors.

I'm not sure I follow this. My understanding is that FileError was designed to make it easy to attach a source location to an error generated while operating on a file, but this patch is adapting it to track the LLVM source location that generated the error, right?

Nov 15 2019, 2:58 PM · Restricted Project

Nov 14 2019

hintonda updated the diff for D70263: [Error] Add source location to Errors.
  • Remove SourceLocationError class and reuse FileError.
Nov 14 2019, 1:48 PM · Restricted Project
hintonda retitled D70263: [Error] Add source location to Errors from [Error] Add SourceLocationError to [Error] Add source location macro.
Nov 14 2019, 1:48 PM · Restricted Project
hintonda updated the diff for D70259: [Error] Add source location to cantFail.
  • Replace macro magic with matching 3-parameter template functions.
Nov 14 2019, 12:52 PM · Restricted Project
hintonda created D70263: [Error] Add source location to Errors.
Nov 14 2019, 11:20 AM · Restricted Project
hintonda created D70259: [Error] Add source location to cantFail.
Nov 14 2019, 11:11 AM · Restricted Project

Nov 11 2019

hintonda committed rGac385ca63fe8: Fix null dereference in yaml::Document::skip (authored by thomasfinch).
Fix null dereference in yaml::Document::skip
Nov 11 2019, 8:58 PM
hintonda closed D69974: Fix null dereference in yaml::Document::skip.
Nov 11 2019, 8:58 PM · Restricted Project

Nov 9 2019

hintonda accepted D69974: Fix null dereference in yaml::Document::skip.

LGTM, but I'll wait a few days to land it for you to give @Bigcheese a chance to comment.

Nov 9 2019, 11:04 AM · Restricted Project

Nov 6 2019

hintonda committed rG405e83689fb4: [CommandLine] Add inline ArgName printing (authored by hintonda).
[CommandLine] Add inline ArgName printing
Nov 6 2019, 8:18 AM
hintonda closed D69501: [CommandLine] Add inline ArgName printing.
Nov 6 2019, 8:18 AM · Restricted Project

Nov 5 2019

hintonda committed rG092452d402d7: YAML parser robustness improvements (authored by thomasfinch).
YAML parser robustness improvements
Nov 5 2019, 9:56 PM
hintonda closed D61608: YAML parser robustness improvements.
Nov 5 2019, 9:56 PM · Restricted Project

Nov 4 2019

hintonda accepted D61608: YAML parser robustness improvements.

I'll go ahead and land this for you tomorrow unless @Bigcheese has any objections.

Nov 4 2019, 6:47 PM · Restricted Project
hintonda added a comment to D69501: [CommandLine] Add inline ArgName printing.

I see you haven't landed this yet. Do you have commit rights? If not, I'd be happy to land it for you.

Nov 4 2019, 4:24 PM · Restricted Project
hintonda requested changes to D61608: YAML parser robustness improvements.
Nov 4 2019, 4:15 PM · Restricted Project

Oct 29 2019

hintonda added a comment to D69501: [CommandLine] Add inline ArgName printing.

LGTM, thanks for the patch!

Oct 29 2019, 10:17 AM · Restricted Project

Oct 28 2019

hintonda added a comment to D69501: [CommandLine] Add inline ArgName printing.

@jhenderson, I added the test.

The only person that has modified llvm/lib/Support/CommandLine.cpp a couple of times in the last year is @hintonda. I added them as a reviewer.

Oct 28 2019, 10:34 AM · Restricted Project

Oct 17 2019

hintonda committed rGecb310b3a7cf: [Error] Make llvm::cantFail include the original error messages (authored by hintonda).
[Error] Make llvm::cantFail include the original error messages
Oct 17 2019, 2:57 PM
hintonda closed D69057: [Error] Make llvm::cantFail include the original error messages.
Oct 17 2019, 2:57 PM · Restricted Project
hintonda updated the diff for D69057: [Error] Make llvm::cantFail include the original error messages.
  • Wrap new login in !NDEBUG.
Oct 17 2019, 2:46 PM · Restricted Project

Oct 16 2019

hintonda created D69057: [Error] Make llvm::cantFail include the original error messages.
Oct 16 2019, 10:18 AM · Restricted Project

Sep 6 2019

hintonda added a comment to D62445: [test] Fix plugin tests.
In D62445#1630518, @NoQ wrote:

I didn't make much progress so far, but i marked the test as // UNSUPPORTED: darwin in rC368765, so the buildbot is now green starting with http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6519/.

Sep 6 2019, 9:07 AM · Restricted Project, Restricted Project

Aug 29 2019

hintonda added a comment to D59754: [Sema] Add c++2a designated initializer warnings.

I'm taking this over to finish it off and submit.

Aug 29 2019, 11:02 PM · Restricted Project, Restricted Project

Aug 5 2019

hintonda added a comment to D62445: [test] Fix plugin tests.
In D62445#1613268, @NoQ wrote:

Ugh, there seems to be one more forgotten buildbot with plugins problems: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6406/consoleText

<snip>

It got suddenly fixed in http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6409/ but then immediately failed again later and it's still failing in a similar manner, and nobody noticed for two months =/

Aug 5 2019, 2:09 PM · Restricted Project, Restricted Project

Aug 4 2019

hintonda added a comment to D62445: [test] Fix plugin tests.
In D62445#1613268, @NoQ wrote:

Ugh, there seems to be one more forgotten buildbot with plugins problems: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6406/consoleText

FAIL: Clang :: Analysis/checker-plugins.c (467 of 14955)
******************** TEST 'Clang :: Analysis/checker-plugins.c' FAILED ********************
Script:
--
: 'RUN: at line 3';   /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/bin/clang -cc1 -internal-isystem /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/lib/clang/9.0.0/include -nostdsysteminc -analyze -analyzer-constraints=range -verify /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/llvm/tools/clang/test/Analysis/checker-plugins.c    -load /Users/buildslave/jenkins/workspace/clang-stage2-cmake-RgSan/clang-build/./lib/SampleAnalyzerPlugin.dylib    -analyzer-checker='example.MainCallChecker'

<snip>

Exit Code: 1

The error message doesn't seem to be very expressive. I also can't reproduce it locally.

It got suddenly fixed in http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6409/ but then immediately failed again later and it's still failing in a similar manner, and nobody noticed for two months =/

I'll keep looking into this but i'd love to hear if you have any immediate thoughts on that :)
Aug 4 2019, 10:24 PM · Restricted Project, Restricted Project

Jul 10 2019

hintonda committed rG43d75f977853: Recommit "[CommandLine] Remove OptionCategory and SubCommand caches from the… (authored by hintonda).
Recommit "[CommandLine] Remove OptionCategory and SubCommand caches from the…
Jul 10 2019, 10:58 AM

Jul 9 2019

hintonda added a comment to D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.

One thought, we should probably evaluate why those tools are using add_library instead of add_llvm_library. The right fix might be to move them over. This is exactly the kind of behavior that add_llvm_library and llvm_add_library were designed to handle.

Jul 9 2019, 3:03 PM · Restricted Project
hintonda updated the diff for D62105: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class..

Make GeneralCategory a ManagedStatic.

Jul 9 2019, 10:31 AM · Restricted Project, Restricted Project
hintonda added a comment to D62105: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class..

Patch is ready, so I'll post as soon as I complete rebase/build/test.

Jul 9 2019, 7:39 AM · Restricted Project, Restricted Project
hintonda reopened D62105: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class..

Reopening to track fix for buildbot failure -- need to make GeneralCategory a ManagedStatic.

Jul 9 2019, 7:26 AM · Restricted Project, Restricted Project

Jul 8 2019

hintonda added a comment to D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.

I don't like the idea of putting this in llvm_update_compile_flags. That function does exactly what its name says, and adding additional behavior to it seems undesirable. Where are we creating libraries with add_library that we need this?

Jul 8 2019, 1:41 PM · Restricted Project

Jul 6 2019

hintonda added a comment to D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.

@beanz, could you take a look? thanks...

Jul 6 2019, 1:21 PM · Restricted Project

Jun 22 2019

hintonda committed rG64b0924531cc: Revert [CommandLine] Remove OptionCategory and SubCommand caches from the… (authored by hintonda).
Revert [CommandLine] Remove OptionCategory and SubCommand caches from the…
Jun 22 2019, 4:34 PM
hintonda committed rGa5b83bc9e3b8: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class. (authored by hintonda).
[CommandLine] Remove OptionCategory and SubCommand caches from the Option class.
Jun 22 2019, 10:25 AM

Jun 21 2019

hintonda added a comment to D63565: [llvm-dwarfdump] Remove unnecessary explicit -h behaviour.

LGTM, but you'll also want to fix llvm/test/Support/check-default-options.txt. It checks your special handling to make sure the default -h support didn't break it.

Thanks, I wasn't aware of that one (and hadn't yet bothered running every lit test for this). Just to be clear, are you recommending that I just delete the bits of that test relating to llvm-dwarfdump, since it will now follow the default process, or do you think there should be something else to replace it? I can replace it with llvm-opt-report, which seems to do something similar to llvm-dwarfdump.

Yes, it can be removed. If you want, you can add it to the ones above that do use the new default -h, but I don't think that's necessary. And yes, if there's another one that does something special, adding it, i.e., s/llvm-dwarfdump/llvm-opt-report/ makes sense.

Okay, I'm going to remove it and then commit, without replacing the removed test part. My initial evaluation was using an out-of-date llvm-opt-report, and I was unable to find any other suitable candidates. I reviewed the other tools, and the only tools I could find with explicit -h help handling are dsymutil and clang-offload-bundler. In both cases, I think the existing handling is a bug (in dsymutil at least, you actually have to provide a positional argument before it will process '-h'!).

Jun 21 2019, 7:30 AM · Restricted Project

Jun 20 2019

hintonda added a comment to D63565: [llvm-dwarfdump] Remove unnecessary explicit -h behaviour.

LGTM, but you'll also want to fix llvm/test/Support/check-default-options.txt. It checks your special handling to make sure the default -h support didn't break it.

Thanks, I wasn't aware of that one (and hadn't yet bothered running every lit test for this). Just to be clear, are you recommending that I just delete the bits of that test relating to llvm-dwarfdump, since it will now follow the default process, or do you think there should be something else to replace it? I can replace it with llvm-opt-report, which seems to do something similar to llvm-dwarfdump.

Jun 20 2019, 8:17 AM · Restricted Project
hintonda accepted D63544: Use object library if cmake supports it.

Finally updated versions - target_sources supports using $<TARGET_OBJECTS:...> since CMake 3.5.0.

Jun 20 2019, 7:33 AM · Restricted Project, Restricted Project
hintonda accepted D63565: [llvm-dwarfdump] Remove unnecessary explicit -h behaviour.

LGTM, but you'll also want to fix llvm/test/Support/check-default-options.txt. It checks your special handling to make sure the default -h support didn't break it.

Jun 20 2019, 7:29 AM · Restricted Project

Jun 19 2019

hintonda added inline comments to D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment.
Jun 19 2019, 10:52 AM · Restricted Project