[PATCH] D61694: Boilerplate for producing XCOFF object files from the PowerPC backend.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a subscriber: rnk.
sfertile added a comment.

In D61694#1576962 <https://reviews.llvm.org/D61694#1576962>, @JamesNagurne wrote:

> Maybe someone can enlighten me as to why the build bots aren't tripping up on this one, but our group is running into this when we pull this commit from the upstream:
>
> In MCStreamer.h, the declaration of the pure virtual EmitSymbolAttribute is:
>
>   virtual bool EmitSymbolAttribute(MCSymbol *Symbol,
>                                    MCSymbolAttr AttrKind,
>                                    StringRef AttrVal = StringRef()) = 0;
>   
>
> In this revision, in MCXCOFFStreamer.h, the declaration is:
>
>   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
>
>
> This is ill-formed, because the 'override' declaration hides the pure virtual member function. Apparently the clang-fast build bots are ok with this, but I'm not sure how. A similar code segment (Compiler Explorer <https://godbolt.org/z/1kZwyy>) fails on every clang compiler I've tried.


Have you modified the signature of EmitSymbolAttribute in your downstream repo? I see

  /// Add the given \p Attribute to \p Symbol.
  virtual bool EmitSymbolAttribute(MCSymbol *Symbol,
                                   MCSymbolAttr Attribute) = 0;

in trunk (line 492) and no one has modified the file for months. A change like this would land with no merge conflicts but still be semantically broken if you have modified the signature. FWIW it bootstraps on trunk at the time I landed it, (and I double checked again right now).

In D61694#1576922 <https://reviews.llvm.org/D61694#1576922>, @stella.stamenova wrote:

> This broke the Windows LLDB bot: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/6493/steps/build/logs/stdio


Sorry about that, I believe @rnk landed a fix in https://reviews.llvm.org/rL365548, the buildbot turned green in the build that picked up that commit. Thanks Reid.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61694/new/

https://reviews.llvm.org/D61694