kbobyrev added inline comments.

================
Comment at: llvm/cmake/modules/FindGRPC.cmake:118
+  # DEPENDS arg is a list of "Foo.proto".
+  foreach(ImportedProto IN LISTS PROTO_DEPENDS)
+    # Foo.proto -> Foo.pb.h
----------------
On line 89 the argument is called `DEPENDS` (also in the comments) but here it 
is called `PROTO_DEPENDS`, I think this is a typo?


================
Comment at: llvm/cmake/modules/FindGRPC.cmake:124
+      ABSOLUTE
+      BASE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+    # Compilation of each generated source depends on ${BINARY}/Foo.pb.h.
----------------
This relies on the protos being generated from the same directory which is 
probably a very restrictive requirement. I don't see a better solution other 
than specifying dirs/full paths to the files in arguments but that does not 
seem nice, too. Maybe add a FIXME?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90215

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

Reply via email to