[llvm] r326810 - [Asm] Fix another layering violation in assmebly macro dumping

[Roger Pau Monné via llvm-commits]

El mar., 6 mar. 2018 17:33, Oliver Stannard <Oliver.Stannard at arm.com>
escribió:

> Ah, I see what you mean.
>
>
>
> The layering violation wasn’t added by my patch, I just turned it into a
> build failure when doing a shared-libraries build.
>
>
>
> The original layering violation it was committed a few weeks ago:
> https://reviews.llvm.org/rL325139, I’ve added the author and committer of
> that patch.
>
>
>
> Oliver
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* 06 March 2018 17:28
> *To:* Pavel Labath
> *Cc:* Oliver Stannard; Pavel Labath via llvm-commits
> *Subject:* Re: [llvm] r326810 - [Asm] Fix another layering violation in
> assmebly macro dumping
>
>
>
> Agreed - this is still a layering violation. Including an MCParser header
> from MC violates the dependencies (MCParser depends on MC, not the other
> way around).
>
> Please revert or fix the original patch.
>
>
>
> On Tue, Mar 6, 2018 at 9:00 AM Pavel Labath <labath at google.com> wrote:
>
> Does that really "fix" the violation? I presume the reason you can use
> T.getString(), but not T.dump() is because the former is inline. However,
> in properly layered code you should not even be able to see the declaration
> of typeof(T) if you do not have it as your dependency...
>
>
>
> On Tue, 6 Mar 2018 at 16:53, Oliver Stannard via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> Author: olista01
> Date: Tue Mar  6 08:51:17 2018
> New Revision: 326810
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326810&view=rev
> Log:
> [Asm] Fix another layering violation in assmebly macro dumping
>
> AsmToken is in the MCParser library, so we can't use its dump function from
> MCAsmMacro in the MC library. Instead, just print the string, which we
> don't
> need the MCParser library for.
>
>
> Modified:
>     llvm/trunk/lib/MC/MCAsmMacro.cpp
>
> Modified: llvm/trunk/lib/MC/MCAsmMacro.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmMacro.cpp?rev=326810&r1=326809&r2=326810&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmMacro.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmMacro.cpp Tue Mar  6 08:51:17 2018
> @@ -25,7 +25,7 @@ void MCAsmMacroParameter::dump(raw_ostre
>        if (!first)
>          OS << ", ";
>        first = false;
> -      T.dump();
> +      OS << T.getString();
>      }
>    }
>    OS << "\n";
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>


Hello,

I'm afraid I'm on vacations without laptop until the 13th. I can look into
it when I'm back. Xen has already merged code related to this feature, so
it would be a shame to revert it now, although I can understand that having
this brokenness is not desirable.

Thanks, Roger.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180307/3b7c79e4/attachment.html>