rjmccall added a comment.

Patch generally looks good, but I agree that it could use some basic tests.  
You don't need to do specifically test all 20 million releases, but make sure 
you cover the first and last in the array as well as something in the middle, 
just to make sure your scans are working right.



================
Comment at: llvm/include/llvm/Support/AVRTargetParser.h:27
+  int Number;
+};
+
----------------
I think the relationship would be clearer if this type were named 
`MCUFamilyInfo`.

More generally, these types and fields could use some comments, like 
"Information about a specific AVR microcontroller release" or "Information 
about a family of AVR microcontrollers" or "Some sort of number that goes 
somewhere and has some kind of meaning, maybe it's reported by hardware or 
something, I dunno".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77221



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

Reply via email to