This is an archive of the discontinued LLVM Phabricator instance.

[GCC] PR23529 Sema part of attrbute abi_tag support
ClosedPublic

Authored by DmitryPolukhin on Feb 24 2016, 3:37 AM.

Details

Summary

Original patch by Stefan Bühler http://reviews.llvm.org/D12834

Difference between original and this one:

  • fixed all comments in original code review
  • added more tests, all new diagnostics now covered by tests
  • moved abi_tag on re-declaration checks to Sema::mergeDeclAttributes where they actually may work as designed
  • clang-format + other stylistic changes

Mangle part will be sent for review as a separate patch.

Diff Detail

Event Timeline

DmitryPolukhin retitled this revision from to [GCC] Sema part of attrbute abi_tag support.
DmitryPolukhin updated this object.
DmitryPolukhin added a reviewer: aaron.ballman.
DmitryPolukhin added subscribers: cfe-commits, stbuehler.
DmitryPolukhin retitled this revision from [GCC] Sema part of attrbute abi_tag support to [GCC] PR23529 Sema part of attrbute abi_tag support.Feb 24 2016, 3:42 AM

Aaron, friendly ping, please take a look!

aaron.ballman edited edge metadata.Mar 2 2016, 5:27 AM
aaron.ballman added a subscriber: aaron.ballman.

I'll definitely take a look as I have the chance. In WG21 meetings all
week, so it may likely be sometime next week.

~Aaron

sberg added a subscriber: sberg.Mar 2 2016, 6:46 AM
sberg added inline comments.
lib/Sema/SemaDeclAttr.cpp
4497

needs to be SmallVector<StringRef, 4> with current trunk

DmitryPolukhin edited edge metadata.
DmitryPolukhin marked an inline comment as done.

Rebase

aaron.ballman added inline comments.Mar 7 2016, 7:27 AM
docs/ItaniumMangleAbiTags.rst
3

ABI tags (since it's an acronym). Elsewhere as well.

31

an <unqualified-name>

50

A namespace does not have any active tags.

51

Comma after "enum)".

53

Comma after "functions".

69

Code examples for these might be useful for the reader.

80

Separate into two sentences instead of using a semicolon.

"Also, for functions, all tags from the..."

84

Replace the ! with a .

Also, an example might be useful for the reader.

90

as an active tag.

include/clang/Basic/AttrDocs.td
1938

to modify the mangled name

1939

the ability to distinguish between different versions

1940

versions supported.

a newer version of a class could have a different set of data members

1941

a different size.

Comma after attribute.

1942

names

1943

Therefore, (note the comma)

the old mangled name

will use the new

include/clang/Basic/DiagnosticSemaKinds.td
2465

Comma after functions.

4179

in a redeclaration

DmitryPolukhin marked 17 inline comments as done.
  • fixed comments
  • rebase with resolving conflicts

Thank you for the comments!

docs/ItaniumMangleAbiTags.rst
84

I'm 100% sure what exactly Stefan meant here and what is intended GCC behavior vs observed behavior, for example:

namespace A {

inline namespace B __attribute__((abi_tag)) {
   struct C { int x; };
}

}

std::string foo() {

struct E {
   __attribute__((abi_tag("X")))
  static A::C bar() {
    // mangle as _ZZ3fooB5cxx11vEN1E3barB1XEv
    // i.e. foo[abi:cxx11]()::E::bar[abi:X]()
    // abi_tag 'B' is missing for bar in GCC 5.3.0
    return A::C();
  }
};
E::bar();

}

Perhaps we should skip this part.

aaron.ballman accepted this revision.Mar 9 2016, 7:10 AM
aaron.ballman edited edge metadata.

LGTM, thank you!

This revision is now accepted and ready to land.Mar 9 2016, 7:10 AM
This revision was automatically updated to reflect the committed changes.
olifre added a subscriber: olifre.Mar 15 2016, 1:51 AM
cecio added a subscriber: cecio.May 9 2016, 2:38 AM