[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Is the attribute considered to be a property of the parameter or a property of the function the parameter is declared in? e.g.,

  void someOtherFunc(void (^cb)(void)) {
    if (something())
      cb();
  }
  
  void barWithCallback(void (^callback)(void) __attribute__((called_once))) {
    someOtherFunc(callback);
  }

Should code like that also diagnose given that `callback` is not called on every code path? From the test cases you have, it looks like you explicitly allow this code without diagnosing it, which suggests the attribute is really a property of the function and not a property of the parameter. This in turn makes me wonder whether a better design is for `-Wcompletion-handler` to diagnose any function with a completion handler-like parameter if the parameter is not called exactly once within the function, and the user marks the functions that should be exempt from the checking rather than marking the parameters that should be checked (or does this have too many false positives in practice)? Alternatively, should the check be implemented as a clang static analyzer check that tracks an annotated parameter across the inter-procedural CFG and diagnoses such code?



================
Comment at: clang/include/clang/Basic/AttrDocs.td:5133-5134
+
+This attribute is useful for API developers who want to double-check if they
+implemented their method correctly.
+
----------------
Oh, I was thinking this attribute was enabling some optimization opportunities or doing something more than just acting as a marker to please check this function's correctness for some properties. This means that the programmer has to annotate their code and enable the warning flag in order for either the attribute or the warning flag to have effect, which feels a bit surprising to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92039