Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(141)

Side by Side Diff: tools/gn/functions.cc

Issue 2586073002: Revert GN declare_args() change. (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « tools/gn/functions.h ('k') | tools/gn/functions_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "tools/gn/functions.h" 5 #include "tools/gn/functions.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 #include <iostream> 8 #include <iostream>
9 #include <utility> 9 #include <utility>
10 10
(...skipping 27 matching lines...) Expand all
38 return true; 38 return true;
39 39
40 *err = Err(block, "Unexpected '{'.", 40 *err = Err(block, "Unexpected '{'.",
41 "This function call doesn't take a {} block following it, and you\n" 41 "This function call doesn't take a {} block following it, and you\n"
42 "can't have a {} block that's not connected to something like an if\n" 42 "can't have a {} block that's not connected to something like an if\n"
43 "statement or a target declaration."); 43 "statement or a target declaration.");
44 err->AppendRange(function->function().range()); 44 err->AppendRange(function->function().range());
45 return false; 45 return false;
46 } 46 }
47 47
48 // This key is set as a scope property on the scope of a declare_args() block,
49 // in order to prevent reading a variable defined earlier in the same call
50 // (see `gn help declare_args` for more).
51 const void *kInDeclareArgsKey = nullptr;
52
53 } // namespace 48 } // namespace
54 49
55
56 bool EnsureNotReadingFromSameDeclareArgs(const ParseNode* node,
57 const Scope* cur_scope,
58 const Scope* val_scope,
59 Err* err) {
60 // If the value didn't come from a scope at all, we're safe.
61 if (!val_scope)
62 return true;
63
64 const Scope* val_args_scope = nullptr;
65 val_scope->GetProperty(&kInDeclareArgsKey, &val_args_scope);
66
67 const Scope* cur_args_scope = nullptr;
68 cur_scope->GetProperty(&kInDeclareArgsKey, &cur_args_scope);
69 if (!val_args_scope || !cur_args_scope || (val_args_scope != cur_args_scope))
70 return true;
71
72 *err = Err(node,
73 "Reading a variable defined in the same declare_args() call.\n"
74 "\n"
75 "If you need to set the value of one arg based on another, put\n"
76 "them in two separate declare_args() calls, one after the other.\n");
77 return false;
78 }
79
80 bool EnsureNotProcessingImport(const ParseNode* node, 50 bool EnsureNotProcessingImport(const ParseNode* node,
81 const Scope* scope, 51 const Scope* scope,
82 Err* err) { 52 Err* err) {
83 if (scope->IsProcessingImport()) { 53 if (scope->IsProcessingImport()) {
84 *err = Err(node, "Not valid from an import.", 54 *err = Err(node, "Not valid from an import.",
85 "Imports are for defining defaults, variables, and rules. The\n" 55 "Imports are for defining defaults, variables, and rules. The\n"
86 "appropriate place for this kind of thing is really in a normal\n" 56 "appropriate place for this kind of thing is really in a normal\n"
87 "BUILD file."); 57 "BUILD file.");
88 return false; 58 return false;
89 } 59 }
(...skipping 290 matching lines...) Expand 10 before | Expand all | Expand 10 after
380 350
381 Introduces the given arguments into the current scope. If they are not 351 Introduces the given arguments into the current scope. If they are not
382 specified on the command line or in a toolchain's arguments, the default 352 specified on the command line or in a toolchain's arguments, the default
383 values given in the declare_args block will be used. However, these defaults 353 values given in the declare_args block will be used. However, these defaults
384 will not override command-line values. 354 will not override command-line values.
385 355
386 See also "gn help buildargs" for an overview. 356 See also "gn help buildargs" for an overview.
387 357
388 The precise behavior of declare args is: 358 The precise behavior of declare args is:
389 359
390 1. The declare_args() block executes. Any variable defined in the enclosing 360 1. The declare_arg block executes. Any variables in the enclosing scope are
391 scope is available for reading, but any variable defined earlier in 361 available for reading.
392 the current scope is not (since the overrides haven't been applied yet).
393 362
394 2. At the end of executing the block, any variables set within that scope 363 2. At the end of executing the block, any variables set within that scope
395 are saved globally as build arguments, with their current values being 364 are saved globally as build arguments, with their current values being
396 saved as the "default value" for that argument. 365 saved as the "default value" for that argument.
397 366
398 3. User-defined overrides are applied. Anything set in "gn args" now 367 3. User-defined overrides are applied. Anything set in "gn args" now
399 overrides any default values. The resulting set of variables is promoted 368 overrides any default values. The resulting set of variables is promoted
400 to be readable from the following code in the file. 369 to be readable from the following code in the file.
401 370
402 This has some ramifications that may not be obvious: 371 This has some ramifications that may not be obvious:
403 372
404 - You should not perform difficult work inside a declare_args block since 373 - You should not perform difficult work inside a declare_args block since
405 this only sets a default value that may be discarded. In particular, 374 this only sets a default value that may be discarded. In particular,
406 don't use the result of exec_script() to set the default value. If you 375 don't use the result of exec_script() to set the default value. If you
407 want to have a script-defined default, set some default "undefined" value 376 want to have a script-defined default, set some default "undefined" value
408 like [], "", or -1, and after the declare_args block, call exec_script if 377 like [], "", or -1, and after the declare_args block, call exec_script if
409 the value is unset by the user. 378 the value is unset by the user.
410 379
411 - Because you cannot read the value of a variable defined in the same 380 - Any code inside of the declare_args block will see the default values of
412 block, if you need to make the default value of one arg depend 381 previous variables defined in the block rather than the user-overridden
413 on the possibly-overridden value of another, write two separate 382 value. This can be surprising because you will be used to seeing the
414 declare_args() blocks: 383 overridden value. If you need to make the default value of one arg
384 dependent on the possibly-overridden value of another, write two separate
385 declare_args blocks:
415 386
416 declare_args() { 387 declare_args() {
417 enable_foo = true 388 enable_foo = true
418 } 389 }
419 declare_args() { 390 declare_args() {
420 # Bar defaults to same user-overridden state as foo. 391 # Bar defaults to same user-overridden state as foo.
421 enable_bar = enable_foo 392 enable_bar = enable_foo
422 } 393 }
423 394
424 Example 395 Example
(...skipping 12 matching lines...) Expand all
437 Value RunDeclareArgs(Scope* scope, 408 Value RunDeclareArgs(Scope* scope,
438 const FunctionCallNode* function, 409 const FunctionCallNode* function,
439 const std::vector<Value>& args, 410 const std::vector<Value>& args,
440 BlockNode* block, 411 BlockNode* block,
441 Err* err) { 412 Err* err) {
442 NonNestableBlock non_nestable(scope, function, "declare_args"); 413 NonNestableBlock non_nestable(scope, function, "declare_args");
443 if (!non_nestable.Enter(err)) 414 if (!non_nestable.Enter(err))
444 return Value(); 415 return Value();
445 416
446 Scope block_scope(scope); 417 Scope block_scope(scope);
447 block_scope.SetProperty(&kInDeclareArgsKey, &block_scope);
448 block->Execute(&block_scope, err); 418 block->Execute(&block_scope, err);
449 if (err->has_error()) 419 if (err->has_error())
450 return Value(); 420 return Value();
451 421
452 // Pass the values from our scope into the Args object for adding to the 422 // Pass the values from our scope into the Args object for adding to the
453 // scope with the proper values (taking into account the defaults given in 423 // scope with the proper values (taking into account the defaults given in
454 // the block_scope, and arguments passed into the build). 424 // the block_scope, and arguments passed into the build).
455 Scope::KeyValueMap values; 425 Scope::KeyValueMap values;
456 block_scope.GetCurrentScopeValues(&values); 426 block_scope.GetCurrentScopeValues(&values);
457 scope->settings()->build_settings()->build_args().DeclareArgs( 427 scope->settings()->build_settings()->build_args().DeclareArgs(
(...skipping 679 matching lines...) Expand 10 before | Expand all | Expand 10 after
1137 } 1107 }
1138 1108
1139 // Otherwise it's a no-block function. 1109 // Otherwise it's a no-block function.
1140 if (!VerifyNoBlockForFunctionCall(function, block, err)) 1110 if (!VerifyNoBlockForFunctionCall(function, block, err))
1141 return Value(); 1111 return Value();
1142 return found_function->second.no_block_runner(scope, function, 1112 return found_function->second.no_block_runner(scope, function,
1143 args.list_value(), err); 1113 args.list_value(), err);
1144 } 1114 }
1145 1115
1146 } // namespace functions 1116 } // namespace functions
OLDNEW
« no previous file with comments | « tools/gn/functions.h ('k') | tools/gn/functions_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698