NoQ added inline comments.

================
Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1064
   // e.g. "{a, b}" represent the qualified names, like "a::b".
   std::vector<const char *> QualifiedName;
   unsigned RequiredArgs;
----------------
a_sidorin wrote:
> Not for this review, but why do we have a vector of `const char *`, not 
> StringRefs?
The constructor accepts `ArrayRef<const char *>` rather than 
`ArrayRef<StringRef>` because `{ "foo", "bar" }` is not a valid initializer for 
`ArrayRef<StringRef>`. The field is the same for simplicity, as converting 
`ArrayRef<const char *>` to a `vector<StringRef>` is annoying and not really 
useful. See also D51390.


================
Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1066
   unsigned RequiredArgs;
+  int Flags;
 
----------------
a_sidorin wrote:
> Is it a good idea to make Flags a bitfield structure?
This would prevent us from passing the flags into the constructor as `CDF_Foo | 
CDF_Bar` etc.- we'll have to specify every field on every line. I wanted to 
make the initializers as concise as possible to avoid line breaks in code like 
D62557.


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:379
+
   if (!II || II != CD.II)
     return false;
----------------
a_sidorin wrote:
> `!II` is never false due to the newly-introduced early return.
Right!


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

https://reviews.llvm.org/D62556



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to