dblaikie added inline comments.

================
Comment at: clang/include/clang/Driver/ToolChain.h:495
+  // compilation uses DWARF5.
+  virtual unsigned GetAdjustedDwarfVersion(unsigned v) const { return v; }
+
----------------
Given these semantics (pass/return the version) maybe "AdjustDwarfVersion" or 
the like might be more suitable (are there other similar functions in this API 
That can/do this sort of pass/return API?).

Alternatively, a narrower/simpler API could be "GetMaxDwarfVersion", say, which 
just returns int_max by default and 2 for NVPTX?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3857
 
+  auto AdjustedDwarfVersion = TC.GetAdjustedDwarfVersion(DWARFVersion);
+
----------------
I worry a bit about having both DWARFVersion and AdjustedDwarfVersion in this 
scope (I'd worry about some codepaths using one when they needed to use the 
other, etc) - if at all possible, would be good to just override DWARFVersion 
with the adjusted value. And probably could/should be up in the "EmitDwarf" 
block above that's handling default dwarf versions and overrides of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92617

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

Reply via email to