[PATCH] Prevent clang from throwing the diagnostics twice.

Hi All,

Clang pop up with the “error: invalid integral value” diagnostics twice ,when you enable the optimization through -O with a non-integer value i.e -O as show below

$ clang -Of -S test.c
error: invalid integral value ‘f’ in ‘-Of’
error: invalid integral value ‘f’ in ‘-Of’

Attached patch fix the issue as

$ clang -Of -S test.c
error: invalid integral value ‘f’ in ‘-Of’

Clang version
clang version 3.4 (http://llvm.org/git/clang.git 26297f57634994b4ae47a0774c372d6944265bb2) (http://llvm.org/git/llvm.git 868e74bdce7a2c49d60e2ef56a077b0aa7f2ba08)

Target: i386-pc-linux-gnu
Thread model: posix

Builded successfully on trunk and is it ok to commit ??

Thanks
~Umesh

clang_changes.patch (1003 Bytes)

llvm_changes.patch (2.02 KB)

Test cases?

Also, this seems a bit heavy-handed for a solution. Why is
getLastArgIntValue being called twice in the first place?

~Aaron

Hi Umesh,
Should the call to StringRef::getAsInteger() be predicated on the
A->getIssuedDiagnostics() call?

I assume the common case would be that we don't emit a diagnostic and thus
the below would be a faster implementation:

if (StringRef(A->getValue()).getAsInteger(10, Res)) {
  if (Diags) {
    if (A->getIssuedDiagnostics() == false){
      A->setIssuedDiagnostics(true);
      Diags->Report(diag::err_drv_invalid_int_value) << A->getAsString(Args)
                                                     << A->getValue();
    }
  }
}

Chad

Test cases?

Yes, please.

Also, this seems a bit heavy-handed for a solution. Why is
getLastArgIntValue being called twice in the first place?

I tend to agree with Aaron here. However, I can't think of a better
solution. Perhaps the issue is that getOptimizationLevel() is being
called multiple times? The call happens in two different static functions
and I don't see a way to easily refactor the code.

Chad

As Chad pointed, it's likely that this requires a non-trivial
refactoring. I don't have particularly bright ideas on solving it, but
I'm uncomfortable adding the flag at the arg level because it kind of
encourages lazy design when handling args. Perhaps looking into ways
the code could be refactored so that getOptimizationLevel() is only
called once (and perhaps the results get cached somewhere) would be a
good approach.

~Aaron

As Chad pointed, it's likely that this requires a non-trivial
refactoring. I don't have particularly bright ideas on solving it, but
I'm uncomfortable adding the flag at the arg level because it kind of
encourages lazy design when handling args. Perhaps looking into ways
the code could be refactored so that getOptimizationLevel() is only
called once (and perhaps the results get cached somewhere) would be a
good approach.

I completely agree with you, Aaron.

Chad

Chad and Aaron,

Thank you for the comments,Let me see how feasible to refactor the code so that getOptimizationLevel() is called once .

Else we need to get cached and reuse the same .

Thanks
~Umesh