[PATCH] D141324: [clang] extend external_source_symbol attribute with the USR clause

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D141324#4041159 <https://reviews.llvm.org/D141324#4041159>, @arphaman wrote:

> In D141324#4039629 <https://reviews.llvm.org/D141324#4039629>, @erichkeane wrote:
>
>> I'm disturbed that the string-literal diagnostic you changed never shows up in the tests.  I suspect this attribute needs significantly better test coverage.
>
> Please elaborate. All the diagnostics have appropriate coverage already.

I'd mistakenly thought that you'd changed the message to add 'name', not moved it.

That said, test coverage of the new feature should be improved.  I don't see anything about the contents of the USR, nor anything with the USR field being dependent.



================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:58
           "for optional message in 'availability' attribute|"
-          "for %select{language|source container}1 name in "
+          "for %select{language name|source container name|USR}1 in "
           "'external_source_symbol' attribute}0">;
----------------
arphaman wrote:
> erichkeane wrote:
> > I don't think adding 'name' to these makes them particularly more readable?  At least from what I can see reading it here.  
> I did not add 'name' here, I moved it from the right hand side as I added a new clause that didn't use name.
Ah! Thanks!  I missed that.


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

https://reviews.llvm.org/D141324