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

[Ben Shi via Phabricator via cfe-commits]

benshi001 added a comment.

Thanks for your suggestion ! I have changed to check avr-1 in `Clang::ConstructJob`, which is much more clear than my previous solution !

Really appreicate. A bit pity is that I have to make big changes to the constructor `AVRToolChain::AVRToolChain`, otherwise the expected error `*** only supports assembly programming` can not rise if `-c` is specified in clang options. Because in previous code, the reading of `FamilyName` is embraced by `!Args.hasArg(options::OPT_c)`, something like

  if (!Args.hasArg(options::OPT_c)) {
      Optional<StringRef> FamilyName = GetMCUFamilyName(CPU);
  }

so I have to re-arrange to code logic.

In D117423#3251110 <https://reviews.llvm.org/D117423#3251110>, @aykevl wrote:

> 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.




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

https://reviews.llvm.org/D117423