[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added inline comments.



Comment at: docs/CodingStandards.rst:514-516
+line of code. Some common editors will automatically remove trailing whitespace
+when saving a file which causes unrelated changes to appear in diffs and
+commits.

Meinersbur wrote:
> Just to note that this policy will prohibit the use of remove 
> trailing-whitespace feature in editors since it makes it impossible to edit 
> the file without also 'editing' any unrelated whitespace. At the same time, 
> since not being able to use the feature, I risk committing trailing 
> whitespace myself (that is, some additional steps are necessary: I use 'git 
> clang-format' which should remove traling whitespace only on edited lines and 
> 'git diff' shows trailing whitespace in red; these are still additional steps 
> that are easy to miss)
I also have editor settings that risk violating this, but I just reduce my 
patdh before submitting. This is a touch annoying with svn, but with got it is 
pretty simple to use `git add -p` and ignore the unnecessary removals if 
trailing whitespace 


https://reviews.llvm.org/D50055



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

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: docs/CodingStandards.rst:514-516
+line of code. Some common editors will automatically remove trailing whitespace
+when saving a file which causes unrelated changes to appear in diffs and
+commits.

Just to note that this policy will prohibit the use of remove 
trailing-whitespace feature in editors since it makes it impossible to edit the 
file without also 'editing' any unrelated whitespace. At the same time, since 
not being able to use the feature, I risk committing trailing whitespace myself 
(that is, some additional steps are necessary: I use 'git clang-format' which 
should remove traling whitespace only on edited lines and 'git diff' shows 
trailing whitespace in red; these are still additional steps that are easy to 
miss)


https://reviews.llvm.org/D50055



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

r338962 - Use Optional instead of unique_ptr; NFC

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Sat Aug  4 18:37:07 2018
New Revision: 338962

URL: http://llvm.org/viewvc/llvm-project?rev=338962=rev
Log:
Use Optional instead of unique_ptr; NFC

Looks like the only reason we use a unique_ptr here is so that we can
conditionally construct a LogicalErrorHandler. It's a small type, and
Optional can do the same thing with 100% fewer heap allocations.

Modified:
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=338962=338961=338962=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Sat Aug  4 18:37:07 2018
@@ -2068,11 +2068,11 @@ AnalysisBasedWarnings::IssueWarnings(sem
   }
 
   // Install the logical handler for -Wtautological-overlap-compare
-  std::unique_ptr LEH;
+  llvm::Optional LEH;
   if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison,
D->getLocStart())) {
-LEH.reset(new LogicalErrorHandler(S));
-AC.getCFGBuildOptions().Observer = LEH.get();
+LEH.emplace(S);
+AC.getCFGBuildOptions().Observer = &*LEH;
   }
 
   // Emit delayed diagnostics.


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

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In https://reviews.llvm.org/D49355#1188520, @aaronpuchert wrote:

> Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do 
> they check? The annotation should certainly not be necessary.
>
> Shouldn't you just use `REQUIRES(!...)` or `EXCLUDES(...)`? If a function 
> locks and unlocks a mutex, I don't see a reason to have annotations on it, 
> other than for preventing double locks. But for that we have negative 
> capabilities.


The purpose here indeed is to avoid double locks. I tried using `EXCLUDES(...)` 
but that doesn't work because `RegisterIsolatePortWithName` 

 calls `LookupIsolatePortByNameUnprotected` 

 which has `EXCLUSIVE_LOCKS_REQUIRED(...)` annotation. I also tried using the 
negative annotation but that reports far too many warnings in the existing code 
which makes it unusable.

I'm fine changing the code, but unless there's a simple workaround I'd still 
argue for a revert, because the change even if correct has broken an existing 
usage pattern that worked fine for a long time before and is used in large 
codebases.


Repository:
  rC Clang

https://reviews.llvm.org/D49355



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

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:1036
+return;
+  // That's it. We can't rule out any more cases with the data we have.
+

rsmith wrote:
> rsmith wrote:
> > lebedev.ri wrote:
> > > rsmith wrote:
> > > > I don't like the overlap between the implicit truncation check and this 
> > > > check. I think you should aim for exactly one of those checks to fire 
> > > > for any given integer conversion. There are the following cases:
> > > > 
> > > >  * Dst is smaller than Src: if the value changes at all (with sign 
> > > > change or without), then the truncation check already catches it, and 
> > > > catching it here does not seem useful
> > > >  * Dst is the same size as Src or larger: sign change is the only 
> > > > problem, and is only possible if exactly one of Src and Dst is signed
> > > > 
> > > > So I think you should bail out of this function if either Src and Dst 
> > > > are both unsigned or both are signed, and also if Src is larger than 
> > > > Dst (because we treat that case as a lossy truncation rather than as a 
> > > > sign change).
> > > > 
> > > > And when you do emit a check here, the only thing you need to check is 
> > > > if the signed value is negative (if so, you definitely changed the 
> > > > sign, and if not, you definitely didn't -- except in the truncation 
> > > > cases that the truncation sanitizer catches).
> > > To be clear: we want to skip emitting in those cases if the other check 
> > > (truncation) is enabled, right?
> > > It does seem to make sense, (and i did thought about that a bit), but i 
> > > need to think about it more..
> > I think we want to skip emitting those checks always (regardless of whether 
> > the other sanitizer is enabled). One way to think of it: this sanitizer 
> > checks for non-truncating implicit integer conversions that change the 
> > value of the result. The other sanitizer checks for truncating implicit 
> > integer conversions that change the value of the result.
> > 
> > I don't see any point in allowing the user to ask to sanitize sign-changing 
> > truncation but not other value-changing truncations. That would lead to 
> > this:
> > ```
> > int a = 0x17fff; // no sanitizer warning
> > int b = 0x18000; // sanitizer warning
> > int c = 0x1; // sanitizer warning
> > int d = 0x2; // no sanitizer warning
> > ```
> > ... which I think makes no sense.
> Hmm, wait, the "truncation" sanitizer doesn't catch this:
> 
> `int a = 0x8000u;`
> 
> ... does it? (Because it looks for cases where the value doesn't round-trip, 
> not for cases where the value was changed by the truncation.)
> 
> 
> I've thought a bit more about the user model and use cases for these 
> sanitizers, and I think what we want is:
> 
>  * a sanitizer that checks for implicit conversions with data loss (the 
> existing truncation sanitizer)
>  * a sanitizer that checks for implicit conversions that change the value, 
> where either the source or destination was signed (approximately what this 
> sanitizer is doing)
> 
> The difference between that and what you have here is that I think the new 
> sanitizer should catch all of these cases:
> 
> ```
> int a = 0x17fff;
> int b = 0x18000;
> int c = 0x1;
> int d = 0x2;
> ```
> 
> ... because while the initializations of `a` and `d` don't change the sign of 
> the result, that's only because they wrap around *past* a sign change.
> 
> So, I think what you have here is fine for the SrcBits <= DstBits case, but 
> for the SrcBits > DstBits case, you should also check whether the value is 
> the same as the original (that is, perform the truncation check).
> 
> In order to avoid duplicating work when both sanitizers are enabled, it'd 
> make sense to combine the two sanitizer functions into a single function and 
> reuse the checks.
Yep, makes sense. I don't think i have followed the recommendations to the 
letter,
but i think the end result is not worse than suggested. Added tests shows how 
it works now.



Comment at: lib/CodeGen/CGExprScalar.cpp:1050-1051
+// NOTE: if it is unsigned, then the comparison is naturally always 
'false'.
+llvm::ICmpInst::Predicate Pred =
+VSigned ? llvm::ICmpInst::ICMP_SLT : llvm::ICmpInst::ICMP_ULT;
+// Get the zero of the same type with which we will be comparing.

rsmith wrote:
> lebedev.ri wrote:
> > rsmith wrote:
> > > If `!VSigned`, the result is a constant `false`; you don't need to emit 
> > > an `icmp` to work that out.
> > Ok, if you insist.
> > I didn't do that in the first place because we will now have an `icmp`
> > where one operand being a constant, so we can simplify it further.
> > And i don't want to complicate this logic if middle-end already handles it 
> > :)
> This becomes a lot simpler with the approach I described in the other comment 
> thread, because you don't need a second `icmp eq` at all.
Humm. So 

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 159187.
lebedev.ri marked 11 inline comments as done.
lebedev.ri added a comment.

Address most of @rsmith review notes.


Repository:
  rC Clang

https://reviews.llvm.org/D50250

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/catch-implicit-integer-conversions-basics.c
  test/CodeGen/catch-implicit-integer-conversions.c
  test/CodeGen/catch-implicit-integer-sign-changes-basics.c
  test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
  test/CodeGen/catch-implicit-integer-sign-changes.c
  test/CodeGen/catch-implicit-integer-truncations-basics.c
  test/CodeGen/catch-implicit-integer-truncations.c
  test/CodeGenCXX/catch-implicit-integer-sign-changes-true-negatives.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -29,22 +29,22 @@
 // CHECK-COVERAGE-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER -implicit-check-not="-fsanitize-address-use-after-scope"
-// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation),?){6}"}}
+// CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|implicit-integer-truncation|implicit-integer-sign-change),?){7}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-RECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fno-sanitize-recover=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-NORECOVER
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=implicit-conversion -fsanitize-trap=implicit-conversion %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-implicit-conversion,CHECK-implicit-conversion-TRAP
-// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}} // ???
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation),?){1}"}}
-// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation),?){1}"}}
+// CHECK-implicit-conversion: "-fsanitize={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-RECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}} // ???
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-NORECOVER-NOT: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP: "-fsanitize-trap={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fsanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
+// CHECK-implicit-conversion-TRAP-NOT: "-fno-sanitize-recover={{((implicit-integer-truncation|implicit-integer-sign-change),?){2}"}}
 
 // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS
 // CHECK-BOUNDS: 

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

Both check-clang and check-clang-tools pass successfully for me on Windows with 
the patch.


Repository:
  rC Clang

https://reviews.llvm.org/D48903



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

[PATCH] D49800: [clang-tidy: modernize] modernize-redundant-void-arg crashes when a function body is in a macro

[Idriss via Phabricator via cfe-commits]

IdrissRio updated this revision to Diff 159179.
IdrissRio added a comment.

Thank you @alexfh. You where right. This update is more general.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49800

Files:
  clang-tidy/modernize/RedundantVoidArgCheck.cpp
  test/clang-tidy/modernize-redundant-void-arg.cpp


Index: test/clang-tidy/modernize-redundant-void-arg.cpp
===
--- test/clang-tidy/modernize-redundant-void-arg.cpp
+++ test/clang-tidy/modernize-redundant-void-arg.cpp
@@ -445,3 +445,49 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in function definition
   // CHECK-FIXES: DefinitionWithNoBody() = delete;
 };
+
+
+
+#define BODY {}
+#define LAMBDA1 [](void){}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA1 [](){}
+
+#define LAMBDA2 [](void)BODY
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA2 []()BODY
+
+#define LAMBDA3(captures, args, body) captures args body
+#define WRAP(...) __VA_ARGS__
+
+#define LAMBDA4 (void)LAMBDA3([],(void),BODY)
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA4 (void)LAMBDA3([],(),BODY)
+
+#define LAMBDA5 []() -> void (*)(void) {return BODY;}
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA5 []() -> void (*)() {return BODY;}
+void lambda_expression_with_macro_test(){
+  (void)LAMBDA1;
+  (void)LAMBDA2;
+  (void)LAMBDA3([], (void), BODY);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: (void)LAMBDA3([], (), BODY);
+
+  LAMBDA4;
+  LAMBDA5;
+   WRAP((void)WRAP(WRAP(LAMBDA3(WRAP([]), WRAP((void)), WRAP(BODY);
+  // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: WRAP((void)WRAP(WRAP(LAMBDA3(WRAP([]), WRAP(()), 
WRAP(BODY);
+
+   (void)WRAP([](void){});
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: (void)WRAP([](){});
+  
+   [](void)BODY;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
+  // CHECK-FIXES: []()BODY;
+}
+
+
+
Index: clang-tidy/modernize/RedundantVoidArgCheck.cpp
===
--- clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -149,6 +149,8 @@
   ProtoToken.getRawIdentifier() == "void") {
 State = SawVoid;
 VoidToken = ProtoToken;
+  } else if (ProtoToken.is(tok::TokenKind::l_paren)) {
+State = SawLeftParen;
   } else {
 State = NothingYet;
   }
@@ -235,9 +237,18 @@
 const MatchFinder::MatchResult , const LambdaExpr *Lambda) {
   if (Lambda->getLambdaClass()->getLambdaCallOperator()->getNumParams() == 0 &&
   Lambda->hasExplicitParameters()) {
-SourceLocation Begin =
-Lambda->getIntroducerRange().getEnd().getLocWithOffset(1);
-SourceLocation End = Lambda->getBody()->getLocStart().getLocWithOffset(-1);
+SourceLocation End, Begin;
+auto SM = Result.SourceManager;
+if (Lambda->getIntroducerRange().getEnd().isMacroID() ||
+Lambda->getBody()->getLocEnd().isMacroID()) {
+  auto TypeLoc =
+  Lambda->getLambdaClass()->getLambdaTypeInfo()->getTypeLoc();
+  End = SM->getSpellingLoc(TypeLoc.getLocEnd());
+  Begin = SM->getSpellingLoc(TypeLoc.getLocStart());
+} else {
+  Begin = Lambda->getIntroducerRange().getEnd().getLocWithOffset(1);
+  End = Lambda->getBody()->getLocStart().getLocWithOffset(-1);
+}
 removeVoidArgumentTokens(Result, SourceRange(Begin, End),
  "lambda expression");
   }


Index: test/clang-tidy/modernize-redundant-void-arg.cpp
===
--- test/clang-tidy/modernize-redundant-void-arg.cpp
+++ test/clang-tidy/modernize-redundant-void-arg.cpp
@@ -445,3 +445,49 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in function definition
   // CHECK-FIXES: DefinitionWithNoBody() = delete;
 };
+
+
+
+#define BODY {}
+#define LAMBDA1 [](void){}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
+// CHECK-FIXES: LAMBDA1 [](){}
+
+#define LAMBDA2 [](void)BODY
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant void argument list in lambda expression 

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

[Bruno Ricci via Phabricator via cfe-commits]

bricci added a comment.

I did both a clean build and an incremental build with and without this patch 
and was not able to see any
meaningful difference in the build time. Suspiciously the build time *with* the 
patch in both cases is a little
less (~0.1% ) than without the patch but this is well within the noise. At 
least I was not able to observe
a significant increase in the build time.


Repository:
  rC Clang

https://reviews.llvm.org/D50261



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

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do 
they check? The annotation should certainly not be necessary.

Shouldn't you just use `REQUIRES(!...)` or `EXCLUDES(...)`? If a function locks 
and unlocks a mutex, I don't see a reason to have annotations on it, other than 
for preventing double locks. But for that we have negative capabilities.


Repository:
  rC Clang

https://reviews.llvm.org/D49355



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

[PATCH] D50294: [Driver] Use -gdwarf-3 by default for FreeBSD

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: brad, emaste, khng300.
Herald added subscribers: cfe-commits, JDevlieghere, aprantl.

The imported binutils in base supports DWARF 3.


Repository:
  rC Clang

https://reviews.llvm.org/D50294

Files:
  lib/Driver/ToolChains/FreeBSD.h
  test/CodeGen/dwarf-version.c
  test/Driver/clang-g-opts.c
  test/Driver/debug-options.c


Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -105,15 +105,15 @@
 // RUN: %clang -### -c -gline-tables-only %s -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_ONLY_DWARF2 %s
 // RUN: %clang -### -c -gline-tables-only %s -target x86_64-pc-freebsd10.0 
2>&1 \
-// RUN: | FileCheck -check-prefix=GLTO_ONLY_DWARF2 %s
+// RUN: | FileCheck -check-prefix=GLTO_ONLY_DWARF3 %s
 // RUN: %clang -### -c -gline-tables-only -g %s -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY %s
 // RUN: %clang -### -c -gline-tables-only -g %s -target x86_64-apple-darwin16 
2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE 
-check-prefix=G_DWARF4 %s
 // RUN: %clang -### -c -gline-tables-only -g %s -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 // RUN: %clang -### -c -gline-tables-only -g %s -target x86_64-pc-freebsd10.0 
2>&1 \
-// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF3 %s
 // RUN: %clang -### -c -gline-tables-only -g %s -target i386-pc-solaris 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
@@ -195,6 +195,10 @@
 // GLTO_ONLY_DWARF2: "-debug-info-kind=line-tables-only"
 // GLTO_ONLY_DWARF2: "-dwarf-version=2"
 //
+// GLTO_ONLY_DWARF3: "-cc1"
+// GLTO_ONLY_DWARF3: "-debug-info-kind=line-tables-only"
+// GLTO_ONLY_DWARF3: "-dwarf-version=3"
+//
 // G_ONLY: "-cc1"
 // G_ONLY: "-debug-info-kind=limited"
 //
@@ -204,6 +208,10 @@
 // G_ONLY_DWARF2: "-debug-info-kind={{standalone|limited}}"
 // G_ONLY_DWARF2: "-dwarf-version=2"
 //
+// G_ONLY_DWARF3: "-cc1"
+// G_ONLY_DWARF3: "-debug-info-kind={{standalone|limited}}"
+// G_ONLY_DWARF3: "-dwarf-version=3"
+//
 // G_STANDALONE: "-cc1"
 // G_STANDALONE: "-debug-info-kind=standalone"
 // G_DWARF4: "-dwarf-version=4"
Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -8,7 +8,7 @@
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target x86_64-pc-freebsd10.0 2>&1 \
-// RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
+// RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF3 %s
 
 // 'g0' is the default. Just sanity-test that it does nothing
 // RUN: %clang -### -S %s -g02>&1 | FileCheck 
--check-prefix=CHECK-WITHOUT-G %s
@@ -26,14 +26,15 @@
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g0 -g -target x86_64-pc-freebsd10.0 2>&1 \
-// RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
+// RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF3 %s
 // RUN: %clang -### -S %s -g0 -g -target i386-pc-solaris 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 
 // CHECK-WITHOUT-G-NOT: -debug-info-kind
 // CHECK-WITH-G: "-debug-info-kind=limited"
 // CHECK-WITH-G: "-dwarf-version=4"
 // CHECK-WITH-G-DWARF2: "-dwarf-version=2"
+// CHECK-WITH-G-DWARF3: "-dwarf-version=3"
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
Index: test/CodeGen/dwarf-version.c
===
--- test/CodeGen/dwarf-version.c
+++ test/CodeGen/dwarf-version.c
@@ -12,7 +12,7 @@
 // RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s 
-isysroot %t | FileCheck %s --check-prefix=VER2
 
 // RUN: %clang -target powerpc-unknown-openbsd -g -S -emit-llvm -o - %s | 
FileCheck %s --check-prefix=VER2
-// RUN: %clang -target powerpc-unknown-freebsd -g -S -emit-llvm -o - %s | 
FileCheck %s --check-prefix=VER2
+// RUN: %clang -target powerpc-unknown-freebsd -g -S -emit-llvm -o - %s | 
FileCheck %s --check-prefix=VER3
 // RUN: %clang -target i386-pc-solaris -g -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=VER2
 //
 // Test what -gcodeview and -gdwarf do on Windows.
Index: lib/Driver/ToolChains/FreeBSD.h
===
--- lib/Driver/ToolChains/FreeBSD.h
+++ lib/Driver/ToolChains/FreeBSD.h
@@ -70,7 +70,7 @@
   const 

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc accepted this revision.
chandlerc added a comment.

This looks really good to me and seems like a nice clarification of historical 
practice. =D Thanks so much for driving an actual documentation update for 
folks that simply would never know about these practices otherwise, I think it 
will help folks a lot.

Maybe double check with Reid and/or Hal to make sure we've all ended up on the 
same page before landing though.


https://reviews.llvm.org/D50055



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

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.




https://reviews.llvm.org/D32747



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