Parser support for string attributes on arguments/return values

[apilipenko]

Summary

Currently string attributes on function arguments/return values can be generated using LLVM API. However they are not supported in parser. So, the following scenario will fail:

• generate function with string attribute using API,
• dump it in LL format,
• try to parse.

Add parser support for string attributes to fix the issue.

[hfinkel]

Currently string attributes on function arguments/return values can be generated using LLVM API.

Which API?

[apilipenko]

In D11058#202557, @hfinkel wrote:

Which API?

Function *function;
function->setAttributes(function->getAttributes().addAttribute(context, i, "attribute"));

[hfinkel]

In D11058#202655, @apilipenko wrote:

In D11058#202557, @hfinkel wrote:

Which API?

Function *function;
function->setAttributes(function->getAttributes().addAttribute(context, i, "attribute"));

I think the intent was for string attributes only to appear in attribute groups (http://llvm.org/docs/LangRef.html#attribute-groups), not as parameter attributes. I don't object to extending this, but we need to have a conversation on llvmdev first about the extension.

[reames]

Given the llvm-dev discussion appears to have petered out without any
objection to the patch itself, LGTM.

Philip

[echristo]

*shrug* I have no objections necessarily either.

-eric

[hfinkel]

LGTM too.

lib/AsmParser/LLParser.h

bool ParseUInt32(unsigned &Val, LocTy &Loc) {
       return ParseUInt32(Val);
     }
     bool ParseUInt64(uint64_t &Val);
     bool ParseUInt64(uint64_t &Val, LocTy &Loc) {
       Loc = Lex.getLoc();
       return ParseUInt64(Val);
     }
 
+    bool ParseStringAttribute(AttrBuilder &B);
+
     bool ParseTLSModel(GlobalVariable::ThreadLocalMode &TLM);
     bool ParseOptionalThreadLocal(GlobalVariable::ThreadLocalMode &TLM);
     bool parseOptionalUnnamedAddr(bool &UnnamedAddr) {
       return ParseOptionalToken(lltok::kw_unnamed_addr, UnnamedAddr);
     }
     bool ParseOptionalAddrSpace(unsigned &AddrSpace);
     bool ParseOptionalParamAttrs(AttrBuilder &B);
     bool ParseOptionalReturnAttrs(AttrBuilder &B);

lib/AsmParser/LLParser.cpp

case lltok::AttrGrpID: {
       if (inAttrGrp) break;
 
       // Save the reference to the attribute group. We'll fill it in later.
       FwdRefAttrGrps.push_back(AttrGrpNum);
       break;
     }
     // Target-dependent attributes:
     case lltok::StringConstant: {
-      std::string Attr = Lex.getStrVal();
-      Lex.Lex();
-      std::string Val;
-      if (EatIfPresent(lltok::equal) &&
-          ParseStringConstant(Val))
+      if (ParseStringAttribute(B))
         return true;
-
-      B.addAttribute(Attr, Val);
       continue;
     }
 
     // Target-independent attributes:
     case lltok::kw_align: {
       // As a hack, we allow function alignment to be initially parsed as an
       // attribute on a function declaration/definition or added to an attribute
       // group and later moved to the alignment field.
bool LLParser::ParseOptionalAddrSpace(unsigned &AddrSpace) {
   AddrSpace = 0;
   if (!EatIfPresent(lltok::kw_addrspace))
     return false;
   return ParseToken(lltok::lparen, "expected '(' in address space") ||
          ParseUInt32(AddrSpace) ||
          ParseToken(lltok::rparen, "expected ')' in address space");
 }
 
+/// ParseStringAttribute
+///   := StringConstant
+///   := StringConstant '=' StringConstant
+bool LLParser::ParseStringAttribute(AttrBuilder &B) {
+  std::string Attr = Lex.getStrVal();
+  Lex.Lex();
+  std::string Val;
+  if (EatIfPresent(lltok::equal) && ParseStringConstant(Val))
+    return true;
+  B.addAttribute(Attr, Val);
+  return false;
+}
+
 /// ParseOptionalParamAttrs - Parse a potentially empty list of parameter attributes.
 bool LLParser::ParseOptionalParamAttrs(AttrBuilder &B) {
   bool HaveError = false;
 
   B.clear();
 
   while (1) {
     lltok::Kind Token = Lex.getKind();
     switch (Token) {
     default:  // End of attributes.
       return HaveError;
+    case lltok::StringConstant: {
+      if (ParseStringAttribute(B))
+        return true;
+      continue;
+    }
     case lltok::kw_align: {
       unsigned Alignment;
       if (ParseOptionalAlignment(Alignment))
         return true;
       B.addAlignmentAttr(Alignment);
       continue;
     }
     case lltok::kw_byval:           B.addAttribute(Attribute::ByVal); break;
bool LLParser::ParseOptionalReturnAttrs(AttrBuilder &B) {
 
   B.clear();
 
   while (1) {
     lltok::Kind Token = Lex.getKind();
     switch (Token) {
     default:  // End of attributes.
       return HaveError;
+    case lltok::StringConstant: {
+      if (ParseStringAttribute(B))
+        return true;
+      continue;
+    }
     case lltok::kw_dereferenceable: {
       uint64_t Bytes;
       if (ParseOptionalDerefAttrBytes(lltok::kw_dereferenceable, Bytes))
         return true;
       B.addDereferenceableAttr(Bytes);
       continue;
     }
     case lltok::kw_dereferenceable_or_null: {

test/Bitcode/attributes.ll

entry:
   ret i8* %foo
 }
 
 ; CHECK: define void @f43() #25
 define void @f43() convergent {
   ret void
 }
 
+; CHECK: define "string_attribute" void @f44(i32 "string_attribute")
+define "string_attribute" void @f44(i32 "string_attribute") {
+  ret void
+}
+
+; CHECK: define "string_attribute_with_value"="value" void @f45(i32 "string_attribute_with_value"="value")
+define "string_attribute_with_value"="value" void @f45(i32 "string_attribute_with_value"="value") {
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
 ; CHECK: attributes #3 = { readonly }
 ; CHECK: attributes #4 = { noinline }
 ; CHECK: attributes #5 = { alwaysinline }
 ; CHECK: attributes #6 = { optsize }
 ; CHECK: attributes #7 = { ssp }