[PATCH] D91979: [Clang][Attr] Introduce the `assume` function attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Dropping the attribute and forcing the attribute to be somehow registered makes various use cases impossible. All we would allow is an integrated compilation where the consumers are available and the FE is aware of them. This is hard for optional LLVM plugins, potentially impossible if you separate the FE and compilation steps, e.g., as you do in LTO builds. I don't see the benefit in dropping the string if it is unknown, the warning is giving the user all the information we have, namely that the FE cannot verify this is a known assumption. Keeping the string attributes doesn't cost us anything and there is no immediate benefit by dropping it (IMHO), it just makes certain compilation scenarios harder/impossible.



================
Comment at: clang/include/clang/Basic/Attr.td:3482
+  let Spellings = [Clang<"assume">];
+  let Subjects = SubjectList<[Function]>;
+  let InheritEvenIfAlreadyPresent = 1;
----------------
aaron.ballman wrote:
> Should this be supported on ObjCMethod declarations as well?
Sure, why not I guess.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1681
+  StringRef Str;
+  if (AL.getNumArgs() == 1 && !S.checkStringLiteralArgumentAttr(AL, 0, Str))
+    return;
----------------
aaron.ballman wrote:
> No need to check the count manually, the common attribute handler should do that for you already.
Copied from the generic templated handler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91979