Update LLVM submodule reference #1594
Conversation
... to get a test fix to support microsoft#1593.
| action.applyTo(config) | ||
| #lit_config.note("Applied '{}' as a result of implicitly detected feature '{}'".format( | ||
| # action.pretty(config, lit_config.params), | ||
| # feature.pretty(config))) |
There was a problem hiding this comment.
Note that the above changes are copied from libcxx's newconfig.py. I commented out the lit_config.note bits because I don't think we want to see this notification spam for every run, but I'm leaving the code here to make it obvious to the next person that this was in fact copied from newconfig.py.
... to get a warning fix.
| feature.enableIn(config) | ||
| lit_config.note( | ||
| "Enabling Lit feature '{}' as a result of parameter '{}'".format(feature.getName(config), param.name)) | ||
| actions = param.getActions(config, lit_config.params) |
There was a problem hiding this comment.
Where is this getActions coming from?
There was a problem hiding this comment.
Upstream. It's a method on Parameter. What used to be getFeature is now getActions, which returns a list of actions to be applied to the configuration instead of simply a feature to add.
(EDIT: fixed the hyperlink I added to actually be to Parameter.getActions instead of Feature.getActions.)
There was a problem hiding this comment.
@ldionne can you explain what the intended purpose of this change was? I looked at the the Phabricator review but it's not immediately obvious.
There was a problem hiding this comment.
The change you want to look at is:
commit d085697013b447abd9a8779eddec606f48626bb9
Author: Louis Dionne <ldionne@apple.com>
Date: Thu Oct 29 16:02:21 2020 -0400
[libc++] Add a new concept of ConfigAction, and use it in the DSL
This will allow adding bare compiler flags through the new
configuration DSL. Previously, this would have required adding
a Lit feature for each such flag.
Differential Revision: https://reviews.llvm.org/D90429
This allows adding things like new substitutions or changing the compiler flags based on whether a command-line parameter is specified. That was previously not possible.
There was a problem hiding this comment.
For example, that change was a pre-requisite for moving the warning flags out of the legacy config:
commit d6e2bac19554a6f877e36d09547b3686b5d7ddb1
Author: Louis Dionne <ldionne@apple.com>
Date: Thu Oct 29 17:30:28 2020 -0400
[libc++] Migrate warning flags to the DSL
This makes us closer to running the test suite on platforms where the
legacy test suite configuration doesn't work.
One notable change after this commit is that the tests will be run with
warnings enabled on GCC too, which wasn't the case before. However,
previous commits should have tweaked the test suite to make sure it
passes with warnings enabled on GCC.
Note that warnings can still be disabled with `--param enable_warnings=False`,
as before.
Differential Revision: https://reviews.llvm.org/D90432
There was a problem hiding this comment.
You folks should consider storing MSVC's configuration in the LLVM monorepo. That way, we wouldn't break you arbitrarily anymore. If your config was visible to me, I'd be happy to change it and notify you when I make changes to libc++'s Lit setup.
|
@CaseyCarter @cbezault I've pushed changes to |
|
Thanks again for taking care of this quarterly-ish update! 😻 🛠️ |
... to get a test fix to support #1593.