ymandel added a comment.
Nice! A few small comments on the headers...
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:56
+
+ /// Assigns `Loc` to `D`.
+ ///
----------------
The term "assignment" is overloaded. :) Maybe instead "Associates `Loc` with
`D`"? Or, expand on the meaning of the assignment? e.g. "Assigns `Loc` as the
storage location of `D`?
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:41
+ /// Creates a storage location appropriate for `Type`. Does not assign a
value
+ /// to the returned storage location in the environment. Never returns null.
+ ///
----------------
here and below. If it never returns null, why not return a ref?
For that matter, all of the pointer parameters with comments "must not be null"
-- can we just make those refs?
================
Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:9
+//
+// This file defines classes that represent elements of the local variable
store
+// and of the heap during dataflow analysis.
----------------
Indent two spaces? (vs 1) same below.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:40-42
+private:
+ Kind LocKind;
+ QualType Type;
----------------
I don't think its required, but LLVM style typically puts private field decls
at the beginning of the class, without the explicit "private:" decl.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:52
+
+ static bool classof(const StorageLocation *Loc) {
+ return Loc->getKind() == Kind::Scalar;
----------------
ref?
================
Comment at: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h:83
+ auto It = Children.find(D);
+ // FIXME: A missing child member means that the sorage location was not
+ // created correctly. Change the following to assert(it != children_.end())
----------------
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:37-38
+
+private:
+ Kind ValKind;
+};
----------------
Move to beginning of class decl?
================
Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:41
+
+/// Class that models an integer.
+class IntegerValue : public Value {
----------------
nit: Here and below. I think that "Models..." would be as good or better than
"Class that models...".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116368/new/
https://reviews.llvm.org/D116368
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
