[PATCH] D54334: [AVR] Automatically link CRT and libgcc from the system avr-gcc

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:44-45
+def warn_drv_avr_family_linking_stdlibs_not_implemented: Warning<
+  "support for linking stdlibs for microcontroller '%0' is not implemented, "
+  "please file an AVR backend bug at http://bugs.llvm.org/ with your mcu name">,
+  InGroup<AVRRtlibLinkingQuirks>;
----------------
This is novel for diagnostics -- we don't have any other diagnostics that recommend filing bugs. This doesn't strike me as something a naive developer will just happen across by accident, so I would probably drop from the comma onward and assume the user knows they can report bugs if they really care.


================
Comment at: lib/Driver/ToolChains/AVR.cpp:40
+
+const std::string PossibleAVRLibcLocations[] = {
+  "/usr/avr",
----------------
Why use `std::string` here when below you use it as a list of `StringRef`s?


================
Comment at: lib/Driver/ToolChains/AVR.cpp:92-94
+      if (!LinkStdlib) {
+        D.Diag(diag::warn_drv_avr_stdlib_not_linked);
+      }
----------------
Elide braces.


================
Comment at: lib/Driver/ToolChains/AVR.cpp:130
+    assert(!CPU.empty() && "CPU name must be known in order to link stdlibs");
+    auto Family = FamilyName.getValue();
+
----------------
I'd drop this and inline it in the use below as `*Family`.


================
Comment at: lib/Driver/ToolChains/AVR.cpp:157-159
+    if (llvm::sys::fs::is_directory(PossiblePath)) {
+      return Optional<std::string>(std::string(PossiblePath));
+    }
----------------
Elide braces.


================
Comment at: lib/Driver/ToolChains/AVR.cpp:162
+
+  return Optional<std::string>();
+}
----------------
`return llvm::None;`


================
Comment at: lib/Driver/ToolChains/AVR.h:45-47
+  Linker(const llvm::Triple &Triple,
+         const ToolChain &TC,
+         bool LinkStdlib)
----------------
Did clang-format produce this? For some reason, it looks off to my eyes.


Repository:
  rC Clang

https://reviews.llvm.org/D54334