[PATCH] D117423: [AVR][clang] Reject non assembly source files for the avr1 family

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

I'm not sure this is the correct location for these checks. You're essentially checking whether the compilation looks like a C/C++ compilation or an assembly compilation based on the flags and the file name. However, the Clang driver already does something like this: it converts the command line arguments and files into a list of jobs to perform. This is done in `Driver::BuildActions`, `Driver::BuildJobs`, `Clang::ConstructJob`, and other places.
I think a better place to do this check is in `Clang::ConstructJob`. There is already something similar here:

https://github.com/llvm/llvm-project/blob/10ed1eca241f893085b8db40138e588e72aaee3a/clang/lib/Driver/ToolChains/Clang.cpp#L4396-L4398

  // C++ is not supported for IAMCU.
  if (IsIAMCU && types::isCXX(Input.getType()))
    D.Diag(diag::err_drv_clang_unsupported) << "C++ for IAMCU";

I think something like this will work, in `Clang::ConstructJob`:

  bool IsAVR = ...
  ...
  
  if (IsAVR) {
      D.Diag(diag::err_drv_clang_unsupported) << "C/C++ for AVR";

There is a different `ConstructJob` for assembly, so this works.



================
Comment at: clang/lib/Driver/ToolChains/AVR.h:22
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
+  std::string AVRMcu;
+
----------------
I think I would have called this `CPU` not `AVRMcu`, but `AVRMcu` is fine.


================
Comment at: clang/test/Driver/avr-mmcu.S:4
+// RUN: %clang -### -target avr -no-canonical-prefixes -mmcu=attiny11 %s -c 2>&1 | FileCheck -check-prefix=CHECKA %s
+// CHECKA-NOT: error: CPU 'attiny11' does not support {{.*}} language mode
+
----------------
Is there a reason why you used `{{.*}}` instead of the actual value?


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

https://reviews.llvm.org/D117423