-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[llvm] simplify and clean-up DemangleConfig.h #149163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@compnerd, @vgvassilev would one of you be willing to merge this patch for me? It is blocking #147564. Thanks! |
This commit creates a library layering violation because Support already depends on Demangle, e.g.
Is there another way you can land this commit w/o creating a cycle? |
## Purpose Simplify `DEMANGLE_` macro definitions in `llvm/Demangle/DemangleConfig.h` for clarity/maintainability. ## Overview * Alias `DEMANGLE_DUMP_METHOD`, `DEMANGLE_FALLTHROUGH`, and `DEMANGLE_UNREACHABLE` macros to their `LLVM_` counterparts defined in `llvm/Support/Compiler.h` * Remove several `DEMANGLE_`-prefixed macros that were only used within `DemangleConfig.h` ## Background * It should be safe for the `Demangle` component library to depend on `Support`, so there is no need for it to maintain copies of macros defined in `llvm/Support/Compiler.h`. * Since the canonical copy `llvm/Demangle/ItaniumDemangle.h` lives under `libcxxabi`, it cannot directly reference the `LLVM_`-prefixed macros so we define `DEMANGLE_`-prefixed aliases. ## Validation * Built llvm-project on Windows with `clang-cl` and MSVC `cl`. * Built llvm-project on Linux with `clang` and `gcc`.
I don't think so, but this change was just cleanup so I will revert it. |
This reverts commit efbe98d.
Reverts #149163 because it introduced a layering violation. Support depends on Demangle: [llvm-project/llvm/lib/Support/Unix/Signals.inc](https://github.com/llvm/llvm-project/blob/324773e238026c5d4f501213678a89bf411e1509/llvm/lib/Support/Unix/Signals.inc#L38) Line 38 in [324773e](324773e)
…50160) Reverts llvm/llvm-project#149163 because it introduced a layering violation. Support depends on Demangle: [llvm-project/llvm/lib/Support/Unix/Signals.inc](https://github.com/llvm/llvm-project/blob/324773e238026c5d4f501213678a89bf411e1509/llvm/lib/Support/Unix/Signals.inc#L38) Line 38 in [324773e](llvm/llvm-project@324773e)
## Purpose Simplify `DEMANGLE_` macro definitions in `llvm/Demangle/DemangleConfig.h` for clarity/maintainability. ## Overview * Alias `DEMANGLE_DUMP_METHOD`, `DEMANGLE_FALLTHROUGH`, and `DEMANGLE_UNREACHABLE` macros to their `LLVM_` counterparts defined in `llvm/Support/Compiler.h` * Remove several `DEMANGLE_`-prefixed macros that were only used within `DemangleConfig.h` ## Background * It should be safe for the `Demangle` component library to depend on `Support`, so there is no need for it to maintain copies of macros defined in `llvm/Support/Compiler.h`. * Since the canonical copy `llvm/Demangle/ItaniumDemangle.h` lives under `libcxxabi`, it cannot directly reference the `LLVM_`-prefixed macros so we define `DEMANGLE_`-prefixed aliases. ## Validation * Built llvm-project on Windows with `clang-cl` and MSVC `cl`. * Built llvm-project on Linux with `clang` and `gcc`.
Reverts llvm#149163 because it introduced a layering violation. Support depends on Demangle: [llvm-project/llvm/lib/Support/Unix/Signals.inc](https://github.com/llvm/llvm-project/blob/324773e238026c5d4f501213678a89bf411e1509/llvm/lib/Support/Unix/Signals.inc#L38) Line 38 in [324773e](llvm@324773e)
Purpose
Simplify
DEMANGLE_
macro definitions inllvm/Demangle/DemangleConfig.h
for clarity/maintainability.Overview
DEMANGLE_DUMP_METHOD
,DEMANGLE_FALLTHROUGH
, andDEMANGLE_UNREACHABLE
macros to theirLLVM_
counterparts defined inllvm/Support/Compiler.h
DEMANGLE_
-prefixed macros that were only used withinDemangleConfig.h
Background
Demangle
component library to depend onSupport
, so there is no need for it to maintain copies of macros defined inllvm/Support/Compiler.h
.llvm/Demangle/ItaniumDemangle.h
lives underlibcxxabi
, it cannot directly reference theLLVM_
-prefixed macros so we defineDEMANGLE_
-prefixed aliases.Validation
clang-cl
and MSVCcl
.clang
andgcc
.